[PATCH] Allow "Closes: nnn" syntax in commit_updater

[Matthijs Kooijman <matthijs@…>]

The commit_updater, which is responsible for adding comments and closing tickets in response to repository commits, uses a regular expression for finding commands in commit messages. Currently, the

Currently, trac's commit_updater uses a regex that allows only one character between the command and the ticket number. This means that "Closes:#123" and "Closes #123" are both valid commands, but "Closes: #123" is not. I find the latter a lot more aesthetically pleasing (especially when used on a separate line at the end of the message, in RFC822 header style).

Would you consider changing the regex to support this? This involves changing commit_updater.py​ as follows:

-    ticket_command = ('(?P<action>[A-Za-z]*).?'
+    ticket_command = ('(?P<action>[A-Za-z]*).?.?'

Note that I haven't tested this using trac 0.12, only using 0.11 and the old-style svn post-commit hook. However, the regexes are identical and the code is similar, so I don't expect any problems here.

[olle.jonsson@…]

And, nit-pickingly, an "action" with 0 characters? Perhaps the * should be a +.

[Christian Boos]

I'd rather see something more explicit like :?\s+ instead of .?.?. Can you please test if that would work for you?

[Matthijs Kooijman <matthijs@…>]

That would indeed be better, but I didn't propose that to keep backwards compatibility (though I can't really imagine any other characters to be used as separators). Looking more closely at the documentation now, it seems that its examples only show the space character as a separator (and don't mention it being optional), so your proposal seems fine to me.

I've just tested with the following regex (so both of the above comments have been applied)

ticket_command =  (r'(?P<action>[A-Za-z]+):?\s+'

I didn't find any problems, everything worked as expected. In particular, I tested the following syntaxes:

References: #1
References #3
References ticket:2
References ticket3

Something that used to work, but does not work anymore is:

References:#2

I'd say this is ugly, but there might be a chance that people actually use this? Changing \s+ to \s* will keep this working (also tested).

[Remy Blank]

Thanks for testing.

[Remy Blank]

Could you please try the following patch?

tracopt/ticket/commit_updater.py

@@ -132 +132 @@
     
     ticket_prefix = '(?:#|(?:ticket|issue|bug)[: ]?)'
     ticket_reference = ticket_prefix + '[0-9]+'
-    ticket_command = ('(?P<action>[A-Za-z]*).?'
-                      '(?P<ticket>%s(?:(?:[, &]*|[ ]?and[ ]?)%s)*)' %
+    ticket_command = (r'(?P<action>[A-Za-z]*)\s*.?\s*'
+                      r'(?P<ticket>%s(?:(?:[, &]*|[ ]?and[ ]?)%s)*)' %
                       (ticket_reference, ticket_reference))
     
     @property

It should allow all the previous syntaxes (the previous regexp allowed any separator character that was not a letter), and allows optional whitespace around the separator.

[Matthijs Kooijman <matthijs@…>]

This seems to work properly. It even allows for fancy indenting now :-)

I've tested these syntaxes:

References: #1
References:#2
References #3
References : #1
Refs       : #2
Refs       : ticket:3

[Remy Blank]

Thanks for testing. Patch applied in [10126].

[Matthijs Kooijman <matthijs@…>]

Awesome, thanks for the quick resolution :-D