Edgewall Software
Modify

Opened 17 years ago

Closed 16 years ago

#7326 closed defect (wontfix)

post-commit-hook doesn't fire if command and ticket number on different lines

Reported by: jenn@… Owned by: John Hampton
Priority: normal Milestone:
Component: version control Version: 0.11
Severity: normal Keywords: trac-post-commit-hook
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

We were having intermittent mysterious failures of post-commit-hook, where the commit succeeded and seemed correctly worded but the changeset wasn't reflected in the ticket. We finally noticed that in the cases where it wasn't working, something had introduced a line break between the "fixes" or "closes" or "refs" and the ticket number.

The obvious workaround is not to break the command and ticket number across lines, but we don't do it intentionally; svn or something is wrapping things at 80 columns for us, thereby introducing line breaks at arbitrary spots.

In consultation on #trac, we came up with the fix of replacing the .? (any single character) at the end of the action with \s* (any whitespace, including newline), which makes a little more sense to me anyway. But that breaks the "fixes:1234" syntax for some reason. This is at the end of this line.

I don't have time to test more permutations, alas. But hopefully this will get someone started on handling multiline commit messages in trac-post-commit-hook.

Attachments (0)

Change History (5)

comment:1 by John Hampton, 17 years ago

Milestone: 0.11.1
Owner: changed from Jonas Borgström to John Hampton
Status: newassigned
Version: 0.11

Can you please try the following patch?

  • contrib/trac-post-commit-hook

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

I think the main question here is: What should the valid separators be? Whitespace and colon? Any others?

comment:2 by Jennifer Drummond <jenn@…>, 17 years ago

Thanks for the new patch! I don't have a current test environment up, but will in a few days (or a couple of weeks) when I get around to upgrading to 0.11 final.

comment:3 by Christian Boos, 16 years ago

Component: generalversion control
Milestone: 0.11.1

I'd say wontfix for this one. Doing this kind of regexp parsing on multiple lines is asking for trouble.

Consider:

(...) can't be fixed

#4567 OTOH could be, with a similar approach.

et voila, you now closed #4567 …

Anyway, the post-commit-hook is meant to be customized, so feel free to do so with your own copy.

in reply to:  3 comment:4 by Emmanuel Blot, 16 years ago

Replying to cboos:

I'd say wontfix for this one. Anyway, the post-commit-hook is meant to be customized, so feel free to do so with your own copy

Agreed. That's why it stands and stays in the /contrib directory.

comment:5 by Remy Blank, 16 years ago

Resolution: wontfix
Status: assignedclosed

Closing as wontfix as suggested in comment:3 and comment:4.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain John Hampton.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from John Hampton 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.