rfc1234 makes a traclink to revision:HEAD

[Tim Hatch <trac@…>]

rf\d{5,} and rfc\d{4,} seem to trigger this. It makes a link that I'd consider invalid (/changeset/fc1234) but that brings up HEAD, as does the link title. Applies to trunk and appears on t.e.o in r4351's message. Writing  rfc:1234 makes the expected link, but I'd expect the (invalid) link to not do anything, rather than link to HEAD.

[cboos]

Well, rfc1234 would be a valid link to the fc1234 revision (hex digits allowed). What is a bit surprising is that the link doesn't have the missing class.

Also, we could consider making this change:

changeset.py

@@ -678 +678 @@
                           title=shorten_line(changeset.message),
                           href=formatter.href.changeset(rev, path))
         except NoSuchChangeset:
-            return html.A(label, class_="missing changeset",
-                          href=formatter.href.changeset(rev, path),
-                          rel="nofollow")
+            return label
 
     def _format_diff_link(self, formatter, ns, params, label):
         def pathrev(path):

[cboos]

Ah, yes, the reason why we get a valid link to the head changeset is that the normalize_rev in the Subversion backend converts any unknown reference to head... in order to do the correct thing when one specifies head, like in /changeset/head URLs.

That's admittedly a bit lazy. We should accept only head and HEAD, eventually also latest but that should be it.

[eblot]

Replying to cboos:

Well, rfc1234 would be a valid link to the fc1234 revision (hex digits allowed).

Is there a use case to allow hexadecimal revision numbers?
If not, I would rather suggest to disallow this format.

Also, we could consider making this change:

I'm not sure to understand the proposal: would that mean that the meaning of the label appears as plain text if the changeset does not exist but becomes a valid revision link once the matching changeset is created?

[cboos]

Replying to eblot:

Replying to cboos:

Well, rfc1234 would be a valid link to the fc1234 revision (hex digits allowed).

Is there a use case to allow hexadecimal revision numbers?

Anything but Subversion ;) And no, I don't think it's a good idea to make that backend dependent. That would involve a lot of changes (making the IWikiSyntaxProvider implementation part of each Connector) for no real benefit.

What could eventually be changed is not allowing hexa digits in the 'r'-style of changeset links. What do users of Darcs, Monotone, Mercurial, etc. backends think about this?

Also, we could consider making this change:

I'm not sure to understand the proposal: would that mean that the meaning of the label appears as plain text if the changeset does not exist but becomes a valid revision link once the matching changeset is created?

In the style of [wiki] ignore_missing_pages, yes.

[eblot]

Replying to cboos:

Anything but Subversion ;) That would involve a lot of changes (making the IWikiSyntaxProvider implementation part of each Connector) for no real benefit.

So I think there is an issue with the rnnnn notation, any word starting with a 'r' is a potential revision: "ref" for example could be interpretated as a revision number ?

I never liked the 'r' syntax because of this potential duality, but it is even worse with hexadecimal number support.

Perhaps the r notation should be reserved to decimal number - [] would be universal, or add a specific marker, such as r:xxx but I really think it's a bad idea that "ref" appears as a revision whereas "refs" appears a word...

I'm not sure to understand the proposal: would that mean that the meaning of the label appears as plain text if the changeset does not exist but becomes a valid revision link once the matching changeset is created?

In the style of [wiki] ignore_missing_pages, yes.

Yeah, but this is an optional feature - users that do not care the meaning of their documents changes over time when another user performs an unrelated action.

Moreover, it's quite easy to notice the CamelCase syntax, where as ref is just a regular word.

Would it be an optional feature? If not, I'm -1 on this patch.

[cboos]

Replying to eblot:

"ref" for example could be interpretated as a revision number ?

No... In the ticket description, Tim was closer to the heuristic actually used. It's this regexp, which is OK most of the time, except of course for the exception case which triggered this ticket ;)

Now, it seems that r<hexadigits> is seldom used for other backends. It's rather directly the changeset id, with usually 12 digits (e.g. 2956948b81f3).

[cboos]

r4354 addresses the points raised in this issue:

• no more r<hexdigits>
• changeset:anystring is no longer an alias for head (only "changeset:head" and a few others are)
• changeset:12345678 remains a "missing changeset" link
• plain hexdigits ids (like 2956948b81f3) are not recognized by default; I'll special case that in the MercurialPlugin.