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

Ignore certain lines in Git messages #126

Closed
rettenbs opened this issue Feb 26, 2020 · 8 comments
Closed

Ignore certain lines in Git messages #126

rettenbs opened this issue Feb 26, 2020 · 8 comments
Labels
enhancement User-facing feature enhancements

Comments

@rettenbs
Copy link

We came across this issue when using Github's "suggestion" feature in pull requests.

The commit message look like this:

Apply suggestions from code review

<custom message>

Co-Authored-By: <user> <email>

The last line is automatically appended by Github and may exceed the max line length if the user name and/or the email is very long.

We would like to ignore the max line length rule for Co-Authored-By: <user> <email> but still check <custom message>.

@jorisroovers
Copy link
Owner

Interesting use-case.

To support this, we'd probably need to do make some changes to the linting code, to check whether a given line should be linted or not.

Ideally I think gitlint should offer some flexibility in how it determines whether a line should be ignored or not. So not just supporting ignoring lines based on matching a given string/regex, but providing a generic mechanism that checks whether a line should be ignored or not. A LineIgnoreRule or something like that.

This requires some more thought on my end, I've had a quick look and this doesn't look very easy to do right.

I'm interested to eventually implement this, but I don't think that will be soon given I just released 0.13.x and am planning a bit of a break now :)

In the meantime, a blunt work-around would be to ignore the body line-length for any commit that contains the Co-Authored-By line.
You can do that using the ignore-by-body rule:

[ignore-by-body]
regex=^Co\-Authored\-By(.*)
ignore=body-max-length

@jorisroovers jorisroovers added the enhancement User-facing feature enhancements label Feb 26, 2020
@jorisroovers
Copy link
Owner

jorisroovers commented Feb 26, 2020

Ok, so I gave this more thought and came up with a much more elegant solution and temporary workaround that are both easy to implement.

Gitlint already has the concept of ConfigurationRules: ignore-by-body and ignore-by-title are currently the only 2 examples we have of those.

Configuration rules are executed before all other rules, and can modify both the lint configuration as well as the commit message on the fly. This last part is important for this use-case. This didn't occur to me before since we don't have a rules as of now that actually modify the commit message. I guess I had some foresight when initially implementing ConfigurationRule rules though :-)

All of this means that supporting this use-case is just a matter of implementing a new rule in gitlint - not that much work (75% of the work is writing tests and docs). I'll do it, but probably not near-term.

Meanwhile, you'll be able to work around this by using the user-defined rule below. Note that this rule pulls some tricks/hacks that are not really supported in general. This is needed because gitlint doesn't yet support user-defined ConfigurationRules, that's still on the wish-list...

# filename: ignore_lines.py
import gitlint
from gitlint.rules import CommitRule, ConfigurationRule

# Monkey patch gitlint so it allows us to define a user-defined ConfigurationRule (workaround)
gitlint.rule_finder.assert_valid_rule_class = lambda *_: True

# For now we'll need to extend from CommitRule for gitlint to find our rule (workaround)
class RemoveBodyLines(ConfigurationRule, CommitRule):

    name = "remove-body-lines"
    id = "CR1"

    def apply(self, config, commit):
        # modify commit message body, drop all lines starting with "Co-Authored-By: "
        commit.message.body = [l for l in commit.message.body if not l.startswith("Co-Authored-By:")]

    # This is needed because we extend from CommitRule (workaround)
    def validate(self, commit):
        return

Execute this like so:

gitlint -e mydir/ignore_lines.py

Or using a .gitlint file with following config:

[general]
extra-path=mydir/ignore_lines.py

Hope this helps!

@nioncode
Copy link

Will the new ConfigurationRule also support the following commit message:

add support for xyz

Closes #234.

At the moment, this triggers B5 Body message is too short, but it would be great to ignore the Closes #{num} / Fixes #{num} etc. from linting.

@jorisroovers
Copy link
Owner

@nioncode Should already be supported by ignore-by-body, see https://jorisroovers.github.io/gitlint/#ignoring-commits

@jorisroovers
Copy link
Owner

jorisroovers commented Mar 31, 2020

@nioncode, just reread your message and realized you probably meant ignoring only the Closes #{num} / Fixes #{num} while still linting the rest of the message. So not ignoring the entire commit (which is what ignore-by-title and ignore-by-body allow you to do).

In that case, yes, this is the exact same use-case IMO and would be covered by this issue. The lines to ignore would be configurable through a regex.

@nioncode
Copy link

nioncode commented Apr 1, 2020

Exactly, I want to only ignore the specific lines. Thanks for the fast response, I'm eagerly awaiting this feature and for now just disable the body-min-length check.

@jorisroovers
Copy link
Owner

Actually @nioncode, thinking about your specific example once more, ignoring that line won't actually make the B5 violation go away. It will just ignore that particular line, but still apply all the rules as usual. In fact, ignoring this line will likely also trigger an B6 body-is-missing violation since other than that line, the commit message body is empty.

I think what you want is to ignore certain rules when the body contains certain lines. Not necessarily ignoring the matching lines themselves - although I can see that you probably also want to do that (that would fit this issue's use-case).

With this in mind, I think the do actually want to use ignore-by-body as of now, but just only ignore specific rules.

[ignore-by-body]
regex=^Closes(.*) # I'll leave the actual regex to you
ignore=B5,B6 # You can specify which rules to ignore

Limitation: It's currently not possible to specify multiple of the same rules (see #113), which means that you wouldn't be able to ignore different rules for different matching regexes. But if the rules you want to ignore are always the same, this should work.

@nioncode
Copy link

nioncode commented Apr 6, 2020

Thanks, I already ignore body-is-missing by default, so I did not think of that particular problem.
Your ignore-by-body method looks nice and i think this would already cover my use case, thanks!

jorisroovers added a commit that referenced this issue Sep 4, 2020
Allows users to dynamically change gitlint's configuration and/or the commit
*before* any other rules are applied.

Relates to #126
jorisroovers added a commit that referenced this issue Sep 4, 2020
Allows users to dynamically change gitlint's configuration and/or the commit
*before* any other rules are applied.

Relates to #126
jorisroovers added a commit that referenced this issue Oct 24, 2020
- IMPORTANT: Gitlint 0.14.x will be the last gitlint release to support Python
  2.7 and Python 3.5, as both are EOL which makes it difficult to keep
  supporting them.
- Python 3.9 support
- New Rule: title-min-length enforces a minimum length on titles
  (default: 5 chars) (#138)
- New Rule: body-match-regex allows users to enforce that the commit-msg body
  matches a given regex (#130)
- New Rule: ignore-body-lines allows users to ignore parts of a commit by
  matching a regex against the lines in a commit message body (#126)
- Named Rules allow users to have multiple instances of the same rule active at
  the same time. This is useful when you want to enforce the same rule multiple
  times but with different options (#113, #66)
- User-defined Configuration Rules allow users to dynamically change gitlint's
  configuration and/or the commit before any other rules are applied.
- The commit-msg hook has been re-written in Python (it contained a lot of
  Bash before), fixing a number of platform specific issues. Existing users
  will need to reinstall their hooks
  (gitlint uninstall-hook; gitlint install-hook) to make use of this.
- Most general options can now be set through environment variables (e.g. set
  the general.ignore option via GITLINT_IGNORE=T1,T2). The list of available
  environment variables can be found in the configuration documentation.
- Users can now use self.log.debug("my message") for debugging purposes in
  their user-defined rules. Debug messages will show up when running
  gitlint --debug.
- Breaking: User-defined rule id's can no longer start with 'I', as those are
  reserved for built-in gitlint ignore rules.
- New RegexOption rule option type for use in user-defined rules. By using the
  RegexOption, regular expressions are pre-validated at gitlint startup and
  compiled only once which is much more efficient when linting multiple commits.
- Bugfixes:
 - Improved UTF-8 fallback on Windows (ongoing - #96)
 - Windows users can now use the 'edit' function of the commit-msg hook (#94)
 - Doc update: Users should use --ulimit nofile=1024 when invoking gitlint
   using Docker (#129)
 - The commit-msg hook was broken in Ubuntu's gitlint package due to a
   python/python3 mismatch (#127)
 - Better error message when no git username is set (#149)
 - Options can now actually be set to None (from code) to make them optional.
 - Ignore rules no longer have "None" as default regex, but an empty regex -
   effectively disabling them by default (as intended).
- Contrib Rules:
 - Added 'ci' and 'build' to conventional commit types (#135)
- Under-the-hood: minor performance improvements (removed some unnecessary
  regex matching), test improvements, improved debug logging, CI runs on pull
  requests, PR request template.

Full Release details in CHANGELOG.md.
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

3 participants