Provide linkable anchors to line indicators in diff view
OpenPublic

Description

It would be of use to be able to link to a specific change of any given diff, especially in the case of a complex diff. For example, in this diff between two versions of the article "Wikipedia" on the English Wikipedia that I selected at random:

https://en.wikipedia.org/w/index.php?title=Wikipedia&diff=594765035&oldid=590925507

the sentence "Also, all pie-charts may be displayed properly in desktop view, but not in mobile view." has been added to the article at what was previously line 449. If I want to direct someone to that particular change, I need to instruct them to either scroll down to the "Line 449" indicator or search for the text in question. This is inefficient.

So, I suggest that every change in the diff (displayed with a "+" or "-" indicator) is accompanied with an anchor link (preferably using an icon of the chain type commonly used online for permanent links). These would be simply calculated as integers starting at 1.

Let's pretend that in the case of the example above, the change I refer to was the 56th in the diff (it wasn't - actually counting what it was would take a long time). An anchor link for that change would generate the following URL:

https://en.wikipedia.org/w/index.php?title=Wikipedia&diff=594765035&oldid=590925507#56


Version: 1.23.0
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=2313

bzimport added a subscriber: wikibugs-l.
bzimport set Reference to bz61486.
Scott created this task.Via LegacyFeb 18 2014, 1:05 PM
PiRSquared17 added a comment.Via ConduitFeb 23 2014, 5:55 AM

Would this disrupt the UI? Should it be enabled for all users? Would the anchor be to the left, right, above, or below the +/- ?

PiRSquared17 added a comment.Via ConduitFeb 23 2014, 5:56 AM

What if we just make the + and - symbols into links (and add hover text)?

Scott added a comment.Via ConduitFeb 23 2014, 10:43 PM

That could work. I do quite like having a definite visible icon for discoverability, but some people might see that as clutter. If there were, I would suggest that they be slightly lower contrast until mouseover, to reduce visual noise. Would be interesting to hear from design people about this.

PiRSquared17 added a comment.Via ConduitFeb 23 2014, 11:27 PM

This might break links if the diffing algorithm is changed significantly, although I do like this idea in general.

Scott added a comment.Via ConduitFeb 24 2014, 11:57 AM

Rats, that's a really good point. Well, I can think of a way to avoid breakage - if the anchor URL specifies what kind of diff is in use, and that particular diff algorithms are always kept available.

In that case, my example URL from above would look something like the following (pretending that our current diff algo is called "diff1"):

https://en.wikipedia.org/w/index.php?title=Wikipedia&diff=594765035&oldid=590925507&difftype=diff1#56

A diff without difftype= would always use the latest algorithm, whatever it may be. Obviously this raises the complexity somewhat; I don't know what the internal policy with regards to diff algorithms is. I would like to imagine that access could still be provided to older ones in the event of changes. Someone who's actually a developer, unlike me, can hopefully shed some light on that.

PiRSquared17 added a comment.Via ConduitFeb 25 2014, 4:01 PM

(re to comment 5)
Sounds like a reasonable idea, but I'm not sure if the devs would like it.

Ricordisamoa added a comment.Via ConduitApr 25 2014, 2:23 PM

Alternative proposal: bug 63707

Aklapper added a comment.Via ConduitAug 16 2014, 10:13 AM

Hi Scott. Thanks for taking the time to report this!
This particular problem has already been reported into our bug tracking system, but please feel free to report any further issues you find.

  • This bug has been marked as a duplicate of bug 2313 ***
Ricordisamoa added a comment.Via ConduitAug 16 2014, 11:57 AM

(In reply to Andre Klapper from comment #9)

AFAIU, bug 2313 and bug 63707 are about linking line indicators to related paragraphs in the content view, while this bug is about anchors to line indicators themselves.

Scott added a comment.Via ConduitAug 18 2014, 11:46 AM

Comment 10 is correct. This bug is not a duplicate of bug 2313, so I'm unsetting that.

gerritbot added a project: Patch-For-Review.Via ConduitJan 16 2015, 11:25 PM
gerritbot added a subscriber: gerritbot.

Change 185569 had a related patch set uploaded (by Nemo bis):
Add linkable anchor #L<N> to (original text) line number in diff

https://gerrit.wikimedia.org/r/185569

Patch-For-Review

gpaumier added a project: user-notice.Via WebThu, Mar 12, 5:46 PM
gpaumier set Security to None.
gpaumier moved this task to Not ready to announce on the user-notice workboard.Via WebThu, Mar 12, 6:08 PM
gerritbot added a comment.Via ConduitFri, Mar 13, 7:06 AM

Change 185569 merged by jenkins-bot:
Add linkable anchor #L<N> to (original text) line number in diff

https://gerrit.wikimedia.org/r/185569

@Nemo_bis @Legoktm I suppose that change does not affect most/all Wikimedia sites, which use the wikidiff2 PHP extension? Why are IDs added only for lines on the left, not on the right? How would you avoid duplicate IDs when a page contains more than one diff? What about the concern raised in T63486#668897 -- that links could potentially break when the diff algorithm, or even the format in which the diff is presented (split vs. unified vs. something else) is changed? What if the new style of diff doesn't use wikitext at all?

Nemo_bis added a comment.Via WebFri, Mar 13, 4:53 PM

PleaseStand, this patch is just a minimal first step which changes the header in which the actual diff content is wrapped (TableDiffFormatter::blockHeader()).
$r = '<tr><td colspan="2" class="diff-lineno" id="L' . $xbeg . '" ><!--LINE ' . $xbeg . "--></td>\n" .

How can a change in this HTML aspect "break" something? Is something relying on this exact HTML structure? That would seem fragile, as the header itself is altered in later stages by DifferenceEngine. Similarly, in what cases two diff tables are combined in one page, causing a possibility of ID reuse?

As for radically different diff engines, probably they wouldn't be using TableDiffFormatter at all. If they do, the only requirement would be that they have a table structure (of course, that's the name) and that they present the diff in a series of chunks headed by a line number: this was already there, nothing new.

Ricordisamoa added a comment.Via WebFri, Mar 13, 6:02 PM

How could this work with T15644: Add automatic line numbering capability to <poem> tag if users choose to show the page content below the diff?

Ricordisamoa awarded a token.Via WebFri, Mar 13, 6:02 PM
gerritbot added a comment.Via ConduitThu, Mar 19, 8:58 AM

Change 197868 had a related patch set uploaded (by Ricordisamoa):
Use more specific and less ambiguous ids for line numbers in diffs

https://gerrit.wikimedia.org/r/197868

Nemo_bis added a comment.Via WebThu, Mar 19, 9:10 AM

How could this work with T15644: Add automatic line numbering capability to <poem> tag if users choose to show the page content below the diff?

Simply by choosing a different ID? Is there some convention which uses L$N as anchor for poems already?

Change 197868 had a related patch set uploaded (by Ricordisamoa):
Use more specific and less ambiguous ids for line numbers in diffs

https://gerrit.wikimedia.org/r/197868

mw-diff-left-l-$N ? Who is going to discover and remember such an anchor? If the anchor is not meant to be found and remembered, what's its purpose? Is any other web application using such a pattern for line diff anchors? Is it valuable or not to use something similar to what gitblit, GitHub and others do?

It seems to me that we don't really want to fix this bug, removing myself.

Nemo_bis removed a subscriber: Nemo_bis.Via WebThu, Mar 19, 9:10 AM
Catrope added a subscriber: Jaredzimmerman-WMF.Via WebWed, Apr 1, 8:52 PM
Catrope added a subscriber: Catrope.

The ID is already implemented, but it's not yet discoverable (i.e. there's no link to it anywhere). That's probably something that a design person should weigh in on.

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.