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

Commit message with a hash sign at the start is not detected #499

Open
syyyr opened this issue May 29, 2023 · 5 comments
Open

Commit message with a hash sign at the start is not detected #499

syyyr opened this issue May 29, 2023 · 5 comments
Labels
enhancement User-facing feature enhancements

Comments

@syyyr
Copy link

syyyr commented May 29, 2023

Hi,

gitlint is not able to detect a commit message that starts with a hash sign. A commit message can begin with a hash sign (for example, by using the -m option for git commit). A usecase for this might be tagging an issue number.

$ git show
commit 0821db153d21cc08755082e83fa247c932490e2c (HEAD -> main)
Author: Václav Kubernát <[email protected]>
Date:   Mon May 29 10:21:16 2023 +0200

    #123 Fix bug
$ gitlint --commits 0821db153d21cc08755082e83fa247c932490e2c
1: T8 Title is too short (0<5)
3: B6 Body message is missing

Can I configure gitlint to not ignore the hashes? I understand that ignoring the hashes is useful for integration with code (commit message) editors (because git commit ignores them if you use the editor), but if I'm linting on our CI pipeline, I'd like to keep the hash signs.

Thanks.

@jorisroovers
Copy link
Owner

Interesting find, TIL that git doesn’t ignore the commentChar when using -m.

This indeed warrants a new option, I’m thinking general.ignore-comment-char. I suspect this is an edge case not many users hit, so I think defaulting to false to maximize backwards compatibility makes sense.

Also, while we might be able to auto-set this option (i.e. true when reading from the repo or stdin, false when specifying --msg-filename), this would require us to allow this option to be set to auto, which no other option in gitlint accepts today. i.e. this would require some extra work to introduce an AutoOption type, which is an interesting thought but going beyond the scope of this issue.

Are you interested in contributing code for this?

2 parts to this:

  1. Add new ignore-comment-char option
  2. Using the option in the code to get the right behavior

Everything will require tests and docs.

@jorisroovers jorisroovers added the enhancement User-facing feature enhancements label Jun 1, 2023
@syyyr
Copy link
Author

syyyr commented Jun 1, 2023

This indeed warrants a new option, I’m thinking general.ignore-comment-char. I suspect this is an edge case not many users hit, so I think defaulting to false to maximize backwards compatibility makes sense.

If I understand correctly, the option should default to true.

Are you interested in contributing code for this?

I'm not sure, if I'm going to have time for this soon, but yes.

@jorisroovers
Copy link
Owner

jorisroovers commented Jun 1, 2023

If I understand correctly, the option should default to true.

You're right!

I'm not sure, if I'm going to have time for this soon, but yes.

Ok, let me know!

@syyyr
Copy link
Author

syyyr commented Jun 5, 2023

I have looked into this a little bit and I don't think I can figure this out on my own:

  1. I found the code which removes lines starting with the commentchar in the GitCommitMessage class. However, I don't know how to propagate the cli flag all the way from the cli parser to there.
  2. I tried searching for the tests that test command line arguments and it seems to me that it's done in test_cli.py. I don't usually code in Python and don't have any experience in Python testing, so the test is very confusing to me, and I'm not sure where to start (for example, how to make a simple assertion that the program at least accepts the option).

I'm still open to coding this, but it won't be an easy thing for me, meaning that it'll take me a while to figure this out (and I probably won't be able to progress without some guidance).

@jorisroovers
Copy link
Owner

Thanks for having a look!

Sounds like it would be a fairly long back-and-forth on this (totally understandable given you usually don't code in Python), in which case I prefer to keep this as-is because I'm limited on available time myself.

I'll probably implement this at some point, just can't commit to a timeline :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement User-facing feature enhancements
Projects
None yet
Development

No branches or pull requests

2 participants