Merge RepoRevisionSyntaxPlugin into core?

[shesek]

When using multiple repositories, all the WikiFormatting / TracLinks has a syntax for linking to a changeset/revision of a specific repository ([123/repo] and [changeset:123/repo]), other than the r123 syntax.

I find the r123 syntax the most readable one and think its weird that it can't be used when using multiple repositories. Therefore, I suggest merging the functionality of ​RepoRevisionSyntaxPlugin (disclosure: this is a plugin I wrote) to core, or using some alternative syntax (repo/r123 maybe?) that allows using this syntax with multiple repositories somehow.

[Remy Blank]

Why use the syntax r123repo, and not e.g. r123/repo, which would be very similar to the [123/repo] syntax? Also, AFAIK the current regexp for the r123 syntax allows hexadecimal revisions (hashes), so without a separator, this might conflict with the repository name (e.g. revision 12 in repository "best" would be written "r12best", which could also be interpreted as revision 12b in repository "est", or revision 12be in repository "st").

So, how about r123/repo (which can conveniently be read as "revision 123 in repository "repo")?

[shesek]

Sure, as I said - any syntax will be fine. In the plugin itself I noted that the only reason I used the r123repo syntax is that Trac parsed the r123 part before my plugin did, so I couldn't implement any `repo/r123' or 'r123/repo' syntax.

I do think that r123/repo is a better choice.

BTW, when does Trac has non-numeric revisions? it seems like 'r1b79' isn't getting parsed to a TracLink

[Remy Blank]

Let's see: r12ab [12ab]

But: r12ab34cd [12ab34cd]

You are right, the hexadecimal revision numbers are only supported with the bracket syntax, and only for 8-digit revisions or longer. The r123 syntax only supports digits. So there should be no ambiguity, but I still prefer r123/repo :)

[shesek]

Yeah, I agree. Any chance this syntax will be used by CommitTicketUpdater too, or make the syntax used by it configurable in TracIni?

[Remy Blank]

Replying to shesek:

Any chance this syntax will be used by CommitTicketUpdater too, or make the syntax used by it configurable in TracIni?

Nah, it looks too ugly with hexadecimal changesets. But you could create a small plugin that subclasses CommitTicketUpdater and overrides make_ticket_comment() to format the ticket comment differently.

[Remy Blank]

Allow r123/path and r123:456/path syntax.

[Remy Blank]

9570-restrict-path-r10006.patch​ adds the r123/path syntax for changesets and r123:456/path for the log.

However, contrary to the [123/path] syntax, the path is limited to characters in [a-zA-Z0-9_/.+-]. Indeed, using a word boundary or whitespace as the end of path would make the comma part of the path in "changed in r123/path, and …". The period could also be removed, for the typical phrase "fixed in r123/path.". If characters outside of that set are needed, the [123/path] syntax should be used instead.

Please test.

[shesek]

Seems to work fine here. I do think that period should not be matched too, and the [123/path] syntax should be used instead on those cases to support "fixed in r123/path." cases. BTW, I never used (nor knew it was possible to use) an entire path and not just the repository name there. Good to know.

[Remy Blank]

Replying to shesek:

I do think that period should not be matched too, and the [123/path] syntax should be used instead on those cases to support "fixed in r123/path." cases.

Agreed. I'll fix that before applying. Thanks for testing!

[Remy Blank]

Patch applied in [10008] (to 0.12-stable by mistake, sorry).