Re: Whitespace at the start of first line of commit
On Fri, 17 Jan 2020, Joel Brobecker wrote: > Do open a GitHub issue if you'd like me to add this check to > the git-hooks. I will likely give it less priority because > you'll have a way to implement it on your on, but I will get to it > eventually. I think https://github.com/AdaCore/git-hooks/issues/7 generally covers all the things we've come up with regarding custom checks *on commit messages*, both those that are clearly GCC-specific (From-SVN:) and those that are more generic for projects where people formerly used ChangeLog content as their commit messages (rejecting a first line looking like a ChangeLog header, rejecting a first line starting with whitespace, rejecting a single-word first line). -- Joseph S. Myers jos...@codesourcery.com
Re: Whitespace at the start of first line of commit
> A quick run of the testsuite reveals that this assumption is made > all over. I am opposed to having this feature be a standard feature > of the git-hooks, so you wouldn't have to add an ad hoc check. > The way I would do it is by enhancing the git_run function to check > for a parameter named "_no_strip" and block the strip() operation > when passed as True. You can then implement your check using the usual > git interface, with the extra "_no_strip=True" parameter. > > Do open a GitHub issue if you'd like me to add this check to > the git-hooks. I will likely give it less priority because > you'll have a way to implement it on your on, but I will get to it > eventually. I apologize. Re-reading myself, I'm mixing various issues and not being clear as a result, let me rephrase: For your immediate need, I am proposing a way to adapt the git_run function so you can use it to implement your check. Another option is to use Python's subprocess module directly, but I think this will be more work than what I suggest. For the longer term, you'll have access to a hook that you can use to call a script to validate an update. In that hook, you'll have total implementation freedom, so you can implement that check there, and not have to worry about that implementation detail in the git-hooks. Also longer term, I can add that check directly in the git-hooks as well. I don't remember anyone ever really making this kind of mistake, but it doesn't mean that it's unlikely either, nor that it would be specific to the GCC community. Hence the merit of having it in git-hooks directly. -- Joel
Re: Whitespace at the start of first line of commit
> Unfortunately, that's not as simple to implement as I'd hoped, because > git.py:git_run removes all leading and trailing whitespace from the output > of the git command it runs, so the code checking commit messages can't > tell whether there was whitespace at the start of the first line (or the > end of the last line, I suppose) at all. I don't know what git commands > the hooks are using that might depend on removing leading whitespace (no > doubt the removal of trailing whitespace is needed in various places to > remove a final newline output by commands giving single-line output). A quick run of the testsuite reveals that this assumption is made all over. I am opposed to having this feature be a standard feature of the git-hooks, so you wouldn't have to add an ad hoc check. The way I would do it is by enhancing the git_run function to check for a parameter named "_no_strip" and block the strip() operation when passed as True. You can then implement your check using the usual git interface, with the extra "_no_strip=True" parameter. Do open a GitHub issue if you'd like me to add this check to the git-hooks. I will likely give it less priority because you'll have a way to implement it on your on, but I will get to it eventually. -- Joel
Re: Whitespace at the start of first line of commit
On Tue, 14 Jan 2020, Joseph Myers wrote: > On Tue, 14 Jan 2020, Jakub Jelinek wrote: > > > (untested), another, suggested by Richard on IRC, would be to reject > > commits where the first line starts with whitespace. > > I'd suggest making the hooks reject whitespace at the start of the first > line of the commit message. Unfortunately, that's not as simple to implement as I'd hoped, because git.py:git_run removes all leading and trailing whitespace from the output of the git command it runs, so the code checking commit messages can't tell whether there was whitespace at the start of the first line (or the end of the last line, I suppose) at all. I don't know what git commands the hooks are using that might depend on removing leading whitespace (no doubt the removal of trailing whitespace is needed in various places to remove a final newline output by commands giving single-line output). -- Joseph S. Myers jos...@codesourcery.com
Re: Whitespace at the start of first line of commit
On Tue, 14 Jan 2020, Jakub Jelinek wrote: > (untested), another, suggested by Richard on IRC, would be to reject > commits where the first line starts with whitespace. I'd suggest making the hooks reject whitespace at the start of the first line of the commit message. -- Joseph S. Myers jos...@codesourcery.com
Whitespace at the start of first line of commit
Hi!
I've noticed that a couple of Jason's commits show up in gcc-cvs
in mutt as:
[gcc r10-5937] ?PR c++/92582 - ICE with member template as requirement.
The ? in there comes from a tab character, the full subject is like
Subject:
=?utf-8?q?=5Bgcc_r10-5937=5D_=09PR_c++/92582_-_ICE_with_member_template_a?=
=?utf-8?q?s_requirement=2E?=
One possibility to deal with this is:
--- hooks/updates/__init__.py 2020-01-12 22:30:37.143193572 +0100
+++ hooks/updates/__init__.py 2020-01-14 11:20:05.746749843 +0100
@@ -315,7 +315,7 @@ class AbstractUpdate(object):
subject = '[%(repo)s%(branch)s] %(subject)s' % {
'repo': self.email_info.project_name,
'branch': branch,
-'subject': commit.subject[:SUBJECT_MAX_SUBJECT_CHARS],
+'subject': commit.subject[:SUBJECT_MAX_SUBJECT_CHARS].strip (),
}
# Generate the body of the email in two pieces:
(untested), another, suggested by Richard on IRC, would be to reject
commits where the first line starts with whitespace.
So, what do we want to do here?
Jakub
