[PATCH] Fragment identifier search

[psuter <petsuter@…>]

As discussed on TracDev/PluginDevelopment, a way to deep link into pages a bit more robust to changes than specifying an anchor like #L123 would be useful.

I propose a JavaScript approach that dynamically searches and scrolls to a term specified in the fragment identifier.

cboos proposed a syntax (based on a vim-ism?) for scrolling to the first match using #/term and to the last match using #?term.

For example the link tags/trac-0.5/setup.py#/import​ would link to line 3 (import sys), while tags/trac-0.5/setup.py#?import​ would link to line 8.

The attached patch implements this by extending the existing search term highlighting. (Note that this makes it available on all searchable pages, like in the source file browser or in the wiki.)

The exact syntax could be changed or extended. Maybe #term:n for the nth and #term:-n for the nth last match?

[psuter <petsuter@…>]

Implements #/forward and #?backward fragment identifier search

[Christian Boos]

I've played a bit with the patch. It worked great though at first I expected the search to be case sensitive, and it was not. I think that in this situation, it's better if the search is case sensitive by default. For example when creating "search anchors", we would certainly always respect the case of a function we want to search for, or respect a capitalized title. And we could add the /i suffix (another vim-ism) if one really would like to have case insensitive search.

Some nitpicks about style: I know our TracDev/CodingStyle doesn't mention Javascript code, but nevertheless it seems we most of the time try to use lower case identifiers for variable names and camelCase only for functions. Btw, I think non anonymous function should be defined as function fooBar() {...} rather than var fooBar = function() {...}.

I'm adding a new version of the patch which adds support for the /i suffix and updates the style (except in the first function, as the style there is currently different, because that function was copied from somewhere else).

[Christian Boos]

applies on top of hash-search.patch​

[Christian Boos]

(there's a small glitch with my change which I just detected; it is apparent in the normal search results as the highlighted terms are turned into lower case…)

[Peter Suter]

Thanks for the feedback.

Replying to cboos:

I think that in this situation, it's better if the search is case sensitive by default.

I agree, but didn't want to make the initial patch too complicated.

Some nitpicks about style: … I know our TracDev/CodingStyle doesn't mention Javascript code

Noted. Maybe we can add a brief summary. Or point to an existing style guide. (There seem to be ​almost infinitely many ​JavaScript ​style ​guides, but none seem to match the Trac conventions too closely as far as I can tell.

[Christian Boos]

Replying to psuter:

.. ​JavaScript …

The style guide from Douglas Crockford seems to be a pretty close match, though. We could add a few local adaptations on that basis. Also, it advises 4 spaces indentation unit, and I would love to switch our code to that convention!

Besides, we could start to document a bit more our Javascript code, there are lots of .js files with just nothing, not even a © Edgewall line…

I'm not sure if the lack of comments and the 2 spaces indentation have a common root in trying to keep those files as small as possible, but if that's the case, we should rather achieve this goal with a minification step (put all our js code in a trac_min.js file, with a make minify step).

​style

Funny, the ​Function Declarations Within Blocks item justifies using var fooBar = function() {...}. But since the jQuery source code doesn't follow this rule, I think the caveats listed there must be of little practical concern, and I think we can indeed use the function fooBar() {...} style if we agree it's nicer that way.

[Peter Suter]

Replying to cboos:

The style guide from Douglas Crockford seems to be a pretty close match, though. We could add a few local adaptations on that basis.

I added some of your and Remy's advice (from #10270) to TracDev/CodingStyle as a first step to that. Expand / expunge as you see fit. :)

we should rather achieve this goal with a minification step (put all our js code in a trac_min.js file, with a make minify step).

The minification topic seems to come up frequently (e.g. in #10245). Instead of a static pre-process, would a dynamic (cached?) processing of scripts passed to add_script be feasible?

[Peter Suter]

Replying to cboos:

(there's a small glitch with my change which I just detected; it is apparent in the normal search results as the highlighted terms are turned into lower case…)

That should be easily fixable:

trac/htdocs/js/search.js

@@ -5 +5 @@
     function highlight(node) {
       if (node.nodeType == 3) { // Node.TEXT_NODE
         var val = node.nodeValue;
-        if (!caseSensitive)
-          val = val.toLowerCase();
-        var pos = val.indexOf(text);
+        var pos = (caseSensitive ? val : val.toLowerCase()).indexOf(text);
         if (pos >= 0 && !$(node.parentNode).hasClass(className)) {
           var span = document.createElement("span");
           span.className = className;

(Or using a new variable for the possibly-lowercase-val, if you dislike the ternary operator.)

Should I commit it like this, or would you wait for more opinions?

[Remy Blank]

+1 from my side for applying.

[Christian Boos]

Final result is fine for me as well (and FWIW, I like using the ternary operator as well; on trunk we even use it for Python with its A if COND else B equivalent).

If you wish at a later point to add the :n extension, it's also fine for me. But what would eventually be even more useful (in the perspective of providing robust links) is the possibility of using a regexp for the search term.

[Peter Suter]

Applied in [10774]. I left the more advanced extensions out for now. Let's see if they'll actually be missed.

Should this functionality be documented somewhere? The closest existing wiki content I could find are TracLinks#Relativelinks and WikiFormatting#SettingAnchors but it doesn't really seem to fit on either of those pages.

[Remy Blank]

Replying to psuter:

Applied in [10774].

Excellent!

Should this functionality be documented somewhere?

Yes, it should. I would add a new section to the "Advanced use of TracLinks" section.

[Peter Suter]

Replying to rblank:

Yes, it should. I would add a new section to the "Advanced use of TracLinks" section.

Should I add it to that (0.12) page you linked, or modify trunk/trac/wiki/default-pages/TracLinks​, or how is this usually done?

I'd merge the first part of Relative Links about anchors with something like this:

=== Link anchors ===

To create a link to a specific anchor in a page, use '#':
{{{
 [#Relativelinks relative links] or [[#Relativelinks|relative links]]
}}}
Displays:
  [#Relativelinks relative links] or [[#Relativelinks|relative links]]

Hint: when you move your mouse over the title of a section, a '¶' character will be displayed. This is a link to that specific section and you can use this to copy the `#...` part inside a relative link to an anchor.

To create a link to the first or last occurrence of a term on a page, use a ''pseudo anchor'' starting with '#/' or '#?':
{{{
 [#/Milestone first occurrence of Milestone] or [#?Milestone last occurrence of Milestone]
}}}
Displays:
 [#/Milestone first occurrence of Milestone] or [#?Milestone last occurrence of Milestone]
This will also highlight all other matches on the linked page. By default only case sensitive matches are considered. To include case insensitive matches append '/i':
{{{
 [#/Milestone/i first occurrence of Milestone or milestone] or [#?Milestone/i last occurrence of Milestone or milestone]
}}}
Displays:
 [#/Milestone/i first occurrence of Milestone or milestone] or [#?Milestone/i last occurrence of Milestone or milestone]

''(since Trac 0.13)''

[Remy Blank]

Replying to psuter:

Should I add it to that (0.12) page you linked, or modify trunk/trac/wiki/default-pages/TracLinks​, or how is this usually done?

You should edit the wiki here on t.e.o. The pages are synced regularly to trunk with a script.

I'd merge the first part of Relative Links about anchors with something like this:

Yes, that looks nice. I notice that the section only shows links to anchors on the page containing the link. Maybe you could also add a few examples of links to anchors on other pages? For example a link to a specific interface in a file in the source browser.

[Peter Suter]

Replying to rblank:

You should edit the wiki here on t.e.o. The pages are synced regularly to trunk with a script.

Neat. I added some examples to TracLinks#Linkanchors.

[Christian Boos]

Replying to psuter:

Replying to rblank:

You should edit the wiki here on t.e.o. The pages are synced regularly to trunk with a script.

Neat. I added some examples to TracLinks#Linkanchors.

Actually this should have been 0.13/TracLinks, as the default page is reserved for documenting the default stable, at least until we're close enough to the release of the next version.

In more details, the workflow for the TracGuide pages goes like this:

• "most of the time"
  • changes for current stable version X are documented in toplevel wiki pages
  • changes on trunk (version X+1) are documented in (X+1)/…
• when approaching the release (usually at the time of the beta)
  • toplevel pages are copied to X/… as a snapshot
  • (X+1)/… pages are copied to toplevel
  • all toplevel pages are reviewed and updated as needed for the new version X

(I usually take care of the copying / moving around - maybe I'll even write a script this time!)

[Remy Blank]

Sorry, my bad advice. Even I wasn't exactly clear how this worked. Maybe you could copy the workflow above into a sub-page of TracDev (if it's not already there, that is)?

[Peter Suter]

Replying to cboos:

Actually this should have been 0.13/TracLinks, as the default page is reserved for documenting the default stable, at least until we're close enough to the release of the next version.

I deleted my previous change (documenting this ticket) from the toplevel page, copied it to 0.13/TracLinks and added my change back there: 0.13/TracLinks#Linkanchors

I hope I got it right this time.

[Christian Boos]

Replying to psuter:

I hope I got it right this time.

Perfect! Thanks.

[Ryan J Ollos]

See also #12756.