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

Support dev comments #115

Closed
drahnr opened this issue Oct 5, 2020 · 8 comments · Fixed by #130
Closed

Support dev comments #115

drahnr opened this issue Oct 5, 2020 · 8 comments · Fixed by #130
Assignees
Labels
enhancement 🦚 New feature or request help wanted 🤝 Extra attention is needed

Comments

@drahnr
Copy link
Owner

drahnr commented Oct 5, 2020

Is your feature request related to a particular use-case?

Dev comments could contain spelling mistakes too.

Describe the solution you'd like to implement/see implemented

A file containing // or /*! .. */ based comments should also be spell checked.

Describe alternatives you've considered

Status quo. Do not check them.

@drahnr drahnr added enhancement 🦚 New feature or request help wanted 🤝 Extra attention is needed hacktoberfest labels Oct 5, 2020
@snasphysicist
Copy link
Contributor

Hi @drahnr I would like to have a go at implementing this improvement please, if that's okay?

@drahnr
Copy link
Owner Author

drahnr commented Oct 9, 2020

@snasphysicist that'd be awesome! The relevant code would mostly around https://github.com/drahnr/cargo-spellcheck/blob/master/src/documentation/literal.rs#L60-L164 - currently syn is used, but this implies that // and /*! */ type comments are not part of the syntax tree, but must be extracted differently.

I assume either applying another rust syntax parser (not sure how many usable there are, rust-analyzer comes to mind) would be an option, manual parsing is probably quite dire.

Feel free to open a PR, I am happy to review this early and often :)

@drahnr
Copy link
Owner Author

drahnr commented Oct 30, 2020

@snasphysicist anything I can help with to get you started?

@snasphysicist
Copy link
Contributor

Hi @drahnr some slow progress is being made - I've been playing with rust_analyzer and rustc to extract the comments' contents from the source files though I haven't yet looked at how to fit this into cargo-spellcheck. I'll link to Rust playground to show how this looks when I've tidied my experiments a bit.

@drahnr
Copy link
Owner Author

drahnr commented Oct 30, 2020

What you described sounds like a good path forward ( the extracted syntax parser lib of ra is https://lib.rs/crates/ra_ap_syntax )

@snasphysicist
Copy link
Contributor

Hmm maybe I am using a slightly out of date version of rust_analyzer. The crate I found was ra_syntax, probably not too different to the one you mention. Anyway I realised that a playground link is not so useful because I guess dependencies aren't supported, so I put a slightly tidied experiment in this gist.

Neither rustc nor rust_analyzer is especially difficult to work with. Unless there are some complexities I've missed I think rust_analyzer is a better candidate because it requires a little less manual work to produce something useful.

If you're happy with the basic idea here, I'll start looking at how to integrate this into the existing cargo-spellcheck source. Let me know if you have any comments/suggestions, thanks!

@drahnr
Copy link
Owner Author

drahnr commented Nov 3, 2020

The gist looks good! I would recommend to take a look at CheckableChunk which is clustered set of Literals to not re-impl it as part of the parser, most of the algorithms built on top of CheckableChunk, so changes there have possible larger fallout effects.

Most of your changes should be in https://github.com/drahnr/cargo-spellcheck/blob/master/src/documentation/literal.rs#L60-L164 (or its vicinity).

Looking forward to your PR!

@snasphysicist
Copy link
Contributor

Hi @drahnr i've opened a WIP PR #130 since I think the feature is getting close to being implemented. Happy to hear any feedback or thoughts. Note that the individual commits might be a bit untidy, I'll tidy them up later where required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🦚 New feature or request help wanted 🤝 Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants