Skip to content

Fix AST parsing for comments #132

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 21, 2022
Merged

Fix AST parsing for comments #132

merged 3 commits into from
Sep 21, 2022

Conversation

SebastienGllmt
Copy link
Contributor

Presumably due to a copy-paste mistake, the condition on this flag was the opposite of what it should be.

You can see this function has 3 cases where it looks at compiler flags. The 3rd flag correctly uses not(feature = "lsp"), but the first flag doesn't.

SebastienGllmt
SebastienGllmt commented Aug 19, 2022
View reviewed changes
SebastienGllmt
SebastienGllmt commented Aug 20, 2022
View reviewed changes
src/parser_tests.rs Outdated

span: (57, 120, 5),
},
comments_before_assignt: None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have coverage of comments_before_assignt since I couldn't think of a way to actually get this to be used since adding a new line before the equal sign in a rule gives a parsing error

i.e.

myrule ; test
  = 234

is not considered valid

@SebastienGllmt
Copy link
Contributor Author

SebastienGllmt commented Aug 26, 2022 •
edited
Loading

Seems the CI fails because of error: package "clap v3.2.17" cannot be built because it requires rustc 1.56.1 or newer, while the currently active rustc version is 1.56.0 which is unrelated to this PR

Seems the fix to that CI issue is included as part of #121

@SebastienGllmt SebastienGllmt mentioned this pull request Aug 30, 2022
Merged
@anweiss
Copy link
Owner

anweiss commented Sep 12, 2022

Thanks for this @SebastienGllmt. #121 has been merged. Feel free to rebase.

@SebastienGllmt
Copy link
Contributor Author

seems there is a new version-related failure in the CI :/

package os_str_bytes v6.3.0 cannot be built because it requires rustc 1.57.0 or newer, while the currently active rustc version is 1.56.1

@anweiss
Copy link
Owner

anweiss commented Sep 12, 2022

Yep. Just bumped the MSRV to 1.57.0 in 2464bea. Try rebasing again.

@anweiss anweiss merged commit fb661a4 into anweiss:main Sep 21, 2022
@SebastienGllmt SebastienGllmt deleted the fix-comment-ast-parsing branch September 22, 2022 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SebastienGllmt @anweiss