mstrap / bugtraq

Specification for linking Git with issue trackers
43 stars 15 forks source link

Special link support #5

Closed mstrap closed 10 years ago

mstrap commented 10 years ago

Sometimes it's nicer to link more than just the actual issue ID in the commit message. We could use logRegexN for that: if there are multiple logRegex, logRegex(N-1) will extract the link part and logRegexN the issue ID from the link.

gitblit commented 10 years ago

I think this feature will quickly become important. If you have multiple sources defined then linking just the integer id doesn't give your eye enough information. Displaying a link as 'issue-id' or "pull request #5" would e better. I'm running SmartGit 5/2071 which seems to support parsing multiple definitions (yeah!) but I haven't gotten this particular spec to work. Looking over the code, it doesn't seem to be part of the unit tests so I am assuming that it isn't yet implemented.

Obviously you have thought more about this than I, but I'd have to say that the logRegexN key names feel odd. Would there ever be a need for more than 2? One to define the link text and one to extract the id from the link text.

mstrap commented 10 years ago

It's actually not yet implemented. Probably you are right and you can extract every piece of String using just one logRegex. TortoiseSVN also only supports either one regular expression or some pre-filter which will be input to the regular expression and they give some good example, why they think a pre-filter is helpful:

http://tortoisesvn.net/docs/nightly/TortoiseSVN_en/tsvn-dug-bugtracker.html

On the other hand, they do not have any special meaning, like links, tied to the pre-filter. So if you (1) need the prefilter and (2) you want to have the ability to customize links, you might need three regular expressions: logFilterRegEx -> logLinkRegEx -> logRegex. Maybe we should call it that way? And have either of the first two being optional?

gitblit commented 10 years ago

I would advocate something like this...

id link

The current implementation - link just the %BUGID% Issue: 5

logRegex = "[I|i]ssue:\\s*(\\d*)"

phrase link

I think logRegex should be a string list (order is preserved and doesn't require int-coded keys).

Issue: 5

logRegex = "([I|i]ssue:\\s*\\d+)"
logRegex = "(\\d+)"

custom link

Your proposal is to also support a 3rd expression to control the link text: BUGTRAQ-5

logRegex = "([I|i]ssue:\\s*\\d+)"
logRegex = "(\\d+)"
displayText = "BUGTRAQ-%BUGID%"

For that we should have a new key. If the key doesn't exist or is empty, we assume logRegex[0] is the displayed text.

mstrap commented 10 years ago

The list approach sounds good. However I don't see how displayText would work, as it seems to replace a String in the commit message? Or is it about a standalone display of the issue ID? -- in this case that makes sense.

mstrap commented 10 years ago

@gitblit can you clarify how displayText would be used?

gitblit commented 10 years ago

I don't have time to code up an example this week, but here is what I was thinking...

right now you track from() and to() boundaries in the commit message to perform substitution/injection. The idea would be to track from() and to() of logregex AND logregex1. logregex1 defines the issueid, logregex defines the entire linkable section, and linkText (or displayText) defines the displayed link text.

url = http://host.com/issue/%BUGID%
logRegex = [I|i]ssue-\\d+
logRegex1 = \\d+
linkText = JIRA-%BUGID%

I haven't worked out all the details... but I'd start from the endpoint.

final String linkText = message.substring(id.getFrom(), id.getTo() + 1);
final String target = issueId.entry.getUrl().replace("%BUGID%", id.getId());
outputHandler.appendLink(linkText, target);
lastIdEnd = id.getTo();

Something like...

final String matchedText = message.substring(id.getFrom(), id.getTo() + 1);
final String linkText;
if (issueid.entry().getLinkText() == null)
{
    linkText = matchedText;
} else
{
    linkText = issueId.entry.getLinkText().replace("%BUGID%", id.getId());
}
final String target = issueId.entry.getUrl().replace("%BUGID%", id.getId());
outputHandler.appendLink(linkText, target);
lastIdEnd = id.getTo();

The id.getFrom() and id.getTo() are incorrect here - but it shouldn't be too hard to get something like this working.

mstrap commented 10 years ago

I've now introduce "loglinktext" which should work as expected.

gitblit commented 10 years ago

Works great. I've merged into Gitblit. And thanks for updating SmartGit to 5.0.4. loglinktext is a very nice usability improvement.