Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent text before the first heading #117

Merged
merged 4 commits into from
Nov 5, 2024
Merged

Conversation

Abeeujah
Copy link
Contributor

closes #115

This PR introduces a new linting rule to enforce a clean document structure by preventing any text from appearing before the first heading in EIPS. This ensures consistency across all EIPS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, how would you feel about heading_first.rs instead? I think headings_only.rs implies that headings are the only node type allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been updated to reflect the desired naming convention.

Comment on lines 25 to 26
let annotation_type = Level::Error;
ctx.report(annotation_type.title("Only Heading is allowed after FrontMatter"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with the other lints, the minimal report should look something like:

Suggested change
let annotation_type = Level::Error;
ctx.report(annotation_type.title("Only Heading is allowed after FrontMatter"))
self.ctx
.annotation_level()
.title("Only Heading is allowed after FrontMatter")
.id(self.slug)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been done to maintain consistency as well

Comment on lines 25 to 26
let annotation_type = Level::Error;
ctx.report(annotation_type.title("Only Heading is allowed after FrontMatter"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this for the error message:

Suggested change
let annotation_type = Level::Error;
ctx.report(annotation_type.title("Only Heading is allowed after FrontMatter"))
ctx.report(annotation_type.title("Nothing is permitted between the preamble and the first heading"))

@Abeeujah
Copy link
Contributor Author

Abeeujah commented Nov 4, 2024

All requested changes has been implemented @SamWilsn

Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last nit, then this looks good to merge.

eipw-lint/src/lints/markdown/heading_first.rs Outdated Show resolved Hide resolved
consistency and improve error message ergonomics.
@Abeeujah
Copy link
Contributor Author

Abeeujah commented Nov 5, 2024

Hi @SamWilsn Gone through your commits on this Pull Request and I must say you had a more elegant solution than I did 🫡 I'm inspired to do more, and get better 💪

@SamWilsn SamWilsn merged commit 262cb86 into ethereum:master Nov 5, 2024
5 checks passed
@SamWilsn
Copy link
Contributor

SamWilsn commented Nov 5, 2024

Not at all! I just plumbed the new lint up, wrote docs, and added line number info. It's still your algorithm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint to prevent text before the first heading
2 participants