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

Sentence case comment linter #534

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

CreepPork
Copy link
Member

When merged this pull request will:

  • Adds a Python linter that will lint all comments in *.sqf, *.cpp and *.hpp files (only // comments are checked)
  • It will check if comments start with a lowercase letter (introduces various inconsistencies in the code)
  • Ignores comments that contain various URLs (http/https)
  • Update the coding guidelines to enforce that all comments start with a uppercase letter
  • Fix existing inconsistencies in the code base regarding comments

For the future (will not be included in this PR):

  • Add a feature that will also check if there is a space or not between the start of the comment and the comment text

@CreepPork CreepPork added ignore changelog Don't add to changelog tools Improves/adds new tools labels Jan 27, 2021
@CreepPork CreepPork added this to the Ongoing milestone Jan 27, 2021
Copy link
Member

@mharis001 mharis001 left a comment

Choose a reason for hiding this comment

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

I dislike this since it does not correctly support the following situations:

  • Multi-line comments that start each line with //. Lines that continued the previous line's sentence are forced to start with a capital, which IMO is a worse style and less readable.
  • Comments that start with command names or config entry names are forced to start with a capital, which makes their casing incorrect.

@CreepPork
Copy link
Member Author

  • Multi-line comments that start each line with //. Lines that continued the previous line's sentence are forced to start with a capital, which IMO is a worse style and less readable.

Yep, this sounds reasonable, and easily done. If comments are chained one after another, then, if the previous comment doesn't end with a ., you could not detect that as a error.

  • Comments that start with command names or config entry names are forced to start with a capital, which makes their casing incorrect.

For this I don't have a very good solution apart from loading everything from SQF into the script, which would make things very ugly. I'd guess it would be easier to just change the sentence structure so that the commands don't start first. If the first point would be implemented it would, for the most part, remove the requirement for this point.

@mharis001
Copy link
Member

Yep, this sounds reasonable, and easily done. If comments are chained one after another, then, if the previous comment doesn't end with a ., you could not detect that as a error.

Yeah, that should work well.

For this I don't have a very good solution apart from loading everything from SQF into the script, which would make things very ugly. I'd guess it would be easier to just change the sentence structure so that the commands don't start first. If the first point would be implemented it would, for the most part, remove the requirement for this point.

Could get the command names from the wiki and parse them using a simple regex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore changelog Don't add to changelog tools Improves/adds new tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants