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

Fix class leading comments order #58

Merged

Conversation

Daniel-Knights
Copy link
Contributor

@Daniel-Knights Daniel-Knights commented Aug 27, 2024

Where class leading comments are moved from the export node to the class node, they are looped in reverse order and pushed. If an exported class has multiple block comments before it (not just JSDoc comments), their order is reversed and the first comment block is processed as the leading comment instead of the last.

If the first block comment isn't a JSDoc comment, the class will be excluded from the generated HTML files. If the first block comment is a JSDoc comment, the class will be included in the generated HTML files, but documented with the first comment instead of the last.

Minimal repro.

@tschaub
Copy link
Member

tschaub commented Aug 28, 2024

Thanks for the proposed change, @Daniel-Knights. It looks like you might have your editor set up to make syntax changes according to some style guidelines that you have configured globally (or otherwise outside of the project directory).

The project's style guidelines are configured with ESLint rules and those rules are enforced by a version of ESLint that is configured in the project.json file. I added a contributing doc with details on getting a development environment set up, running the tests, and conforming with the linter rules. Let us know if it would be useful to have additional detail in that doc.

Linter issues aside, it would be great if you could add a test that demonstrates what this fix is about.

@Daniel-Knights
Copy link
Contributor Author

Daniel-Knights commented Sep 5, 2024

Added a test @tschaub. To clarify, in the added test /* Comment */ and /** Doclet */ get swapped with .push, and /* Comment */ is considered the leading comment of the class instead of /** Doclet */. The fix is to use .unshift instead to ensure they remain in the correct order.

Appreciate it's a bit of a confusing explanation.

Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

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

Looks good now. Please squash the first two commits (or all three, if that's easier for you) into one to avoid the back and forth on trailing commas.

@ahocevar ahocevar merged commit c0a0dcb into openlayers:main Sep 13, 2024
1 check passed
@ahocevar
Copy link
Member

Thanks, @Daniel-Knights

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.

3 participants