Edgewall Software
Modify

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#9615 closed enhancement (fixed)

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

Reported by: Matthijs Kooijman <matthijs@…> Owned by: Remy Blank
Priority: normal Milestone: 0.12.1
Component: ticket system Version: 0.13dev
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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.

Attachments (0)

Change History (8)

comment:1 by olle.jonsson@…, 12 years ago

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

comment:2 by Christian Boos, 12 years ago

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

comment:3 by Matthijs Kooijman <matthijs@…>, 12 years ago

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).

comment:4 by Remy Blank, 12 years ago

Milestone: 0.12.1
Owner: set to Remy Blank

Thanks for testing.

comment:5 by Remy Blank, 12 years ago

Could you please try the following patch?

  • tracopt/ticket/commit_updater.py

    diff --git a/tracopt/ticket/commit_updater.py b/tracopt/ticket/commit_updater.py
    a b  
    132132   
    133133    ticket_prefix = '(?:#|(?:ticket|issue|bug)[: ]?)'
    134134    ticket_reference = ticket_prefix + '[0-9]+'
    135     ticket_command = ('(?P<action>[A-Za-z]*).?'
    136                       '(?P<ticket>%s(?:(?:[, &]*|[ ]?and[ ]?)%s)*)' %
     135    ticket_command = (r'(?P<action>[A-Za-z]*)\s*.?\s*'
     136                      r'(?P<ticket>%s(?:(?:[, &]*|[ ]?and[ ]?)%s)*)' %
    137137                      (ticket_reference, ticket_reference))
    138138   
    139139    @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.

comment:6 by Matthijs Kooijman <matthijs@…>, 12 years ago

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

comment:7 by Remy Blank, 12 years ago

Resolution: fixed
Status: newclosed

Thanks for testing. Patch applied in [10126].

comment:8 by Matthijs Kooijman <matthijs@…>, 12 years ago

Awesome, thanks for the quick resolution :-D

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.