Confusing diff for spatially disparate changes
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Izno
	Sep 15 2023, 6:34 PM

Description

In this diff:

displays. But in wikEdDiff,

displays, which has blocked sections. And in 'block' diff, the geography is also better because it includes the separate section with line #:

There should be some sort of indication (even?) in inline mode that those two sections are geographically distant (perhaps just an <hr>). It was a very confusing diff when I looked at it this morning.

Acceptance criteria

Inline diffs should surface line numbers as provided by wikidiff2
- These are surfaced if there are for example changes across multiple paragraphs, or if line numbers changed significantly from the old revision versus the new.
- The logic of when to show line numbers is not part of this task. We're using the default behaviour that already exist in wikidiff2.
The line numbers should be in the form of Line 5 ⟶ 10:, where 5 is the previous line number, and 10 is the line as of the new revision.
If the line numbers haven't changed (i.e. both the previous and new line numbers are 5), it should use the shorter message Line 5:
If the reducedLineNumbers option is true (as it is on our cluster), you shouldn't ever see Line 1:, but you may see for example Line 1 ⟶ 5.
- TBD: How to turn on this mode? Is it a config setting?

QA Results - Beta

AC	Status	Details
1	✅AC1: Inline diffs should surface line numbers as provided by wikidiff2 here
2	✅AC2: The line numbers should be in the form of Line 5 ⟶ 10:, where 5 is the previous line number, and 10 is the line as of the new revision.
3	✅AC3: If the line numbers haven't changed (i.e. both the previous and new line numbers are 5), it should use the shorter message Line 5:
4	✅AC4: If the `reducedLineNumbers` option is true (as it is only for MediaWiki-extensions-Translate), you shouldn't ever see Line 1:, but you may see for example Line 1 ⟶ 5.

Details

Subject	Repo	Branch	Lines +/-
diffs: add line number headings to inline diffs	mediawiki/core	wmf/1.41.0-wmf.30	+36 -7
diffs: add line number headings to inline diffs	mediawiki/core	REL1_41	+36 -7
diffs: add line number headings to inline diffs	mediawiki/core	master	+36 -7

Customize query in gerrit

Related Objects

Mentioned In: T117279: [EPIC] Core should provide inline diffs as well as side by side (Move InlineDifferenceEngine into core / remove MobileDiff)
T310833: Uploaded files via the drag-and-drop are defaulting to private-access
T347013: Include line numbers for every line of the inline diffs on code pages
Mentioned Here: rMW625f20857a03: Localisation updates from https://translatewiki.net.
T310833: Uploaded files via the drag-and-drop are defaulting to private-access
T347013: Include line numbers for every line of the inline diffs on code pages

Event Timeline

Izno created this task.Sep 15 2023, 6:34 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 15 2023, 6:34 PM

Izno updated the task description. (Show Details)Sep 15 2023, 6:46 PM

Izno added a project: MediaWiki-Page-diffs.Sep 15 2023, 8:12 PM

Izno updated the task description. (Show Details)Sep 15 2023, 8:34 PM

Umherirrender edited projects, added wikidiff2; removed MediaWiki-Page-diffs.Sep 16 2023, 8:45 PM

Pols12 subscribed.Sep 20 2023, 1:51 PM

@Umherirrender , it is a design issue, not a backend diff calculation issue.

In y opinion, the best design to solve this issue is to add line numbers which quickly help to find the line m4in the editor.

Pols12 edited projects, added MediaWiki-Page-diffs; removed wikidiff2.Sep 20 2023, 1:58 PM

In T346460#9183129, @Pols12 wrote:

@Umherirrender , it is a design issue, not a backend diff calculation issue.

It was not clear for me from reading that it is only about the styling, not about detecting the differences. Switching the tags in that case is correct. Thanks.

TheresNoTime added a project: Community-Tech (CommTech-Kanban).Sep 20 2023, 6:05 PM

Erutuon subscribed.Sep 21 2023, 3:52 AM

In T346460#9183129, @Pols12 wrote:

In y opinion, the best design to solve this issue is to add line numbers which quickly help to find the line m4in the editor.

A separate ask. I filed T347013: Include line numbers for every line of the inline diffs on code pages

Benwing2 subscribed.Sep 22 2023, 10:19 PM

Eek! We'll get this fixed, but since it will likely require a change to Wikidiff2, it won't be a quick fix. Fortunately the logic is already written; there's a .mw-diff-inline-header above each section, and in it is an HTML comment containing the line numbers. Some simple JS could surface this but that feels too hacky in my opinion.

Does not require a change to Wikidiff2, so I'm hoping to get a fix out this week.

So with inline, the two line numbers don't make much sense. I think it's confusing to list both, so I'm thinking of only listing the new line number? I imagine that's usually what people would want (but it's possible someone would want to edit the old revision too).

Something like this:

Screenshot from 2023-10-02 19-14-03.png (911×1 px, 253 KB)

Thoughts? I think going with just the above is satisfactory, but I'm a bit interested in taking it a step further and maybe adding a border or something, similar to what's done for WikEd (i.e. F37727816)?

Pinging @JSengupta-WMF for input.

Change 962724 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/core@master] diffs: add line number headings to inline diffs

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

gerritbot added a project: Patch-For-Review.Oct 2 2023, 11:23 PM

New line numbers alone is fine. Could also do something like "Line X became line Y".

Even if we had line numbers in all namespaces for everyone in the editor, I'd probably still Ctrl + F to get to a location in a diff, since that's basically how I do it even with line numbers (in Module namespace). So that probably tends toward "were never all that useful but if someone thinks they are I don't personally mind".

Umherirrender unsubscribed.Oct 3 2023, 1:20 PM

@MusikAnimal I like the proposal of @Izno "Line X became Line Y". Can we explore that?

In T346460#9224140, @JSengupta-WMF wrote:

@MusikAnimal I like the proposal of @Izno "Line X became Line Y". Can we explore that?

Agreed, that's a good idea!

Here's two candidates, personally I like the arrow more as it's easier on the eyes:

Screenshot from 2023-10-05 17-41-23.png (1×1 px, 328 KB)

Screenshot from 2023-10-05 17-41-32.png (1×1 px, 332 KB)

In T346460#9229657, @MusikAnimal wrote:

Make those public. :)

Let's keep the 1st version with arrow.

Let's keep the 1st version with arrow.

Sounds good. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/962724 is now ready for review.

In T346460#9230106, @Izno wrote:

In T346460#9229657, @MusikAnimal wrote:

F37988241
F37988240

Make those public. :)

(Belated) done! So new lesson learned... When *editing* an existing Phab post, this bug happens. Normally it's only with drag and drop. I've commented at T310833.

MusikAnimal mentioned this in T310833: Uploaded files via the drag-and-drop are defaulting to private-access.Oct 9 2023, 11:16 PM

Change 962724 merged by jenkins-bot:

[mediawiki/core@master] diffs: add line number headings to inline diffs

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

Maintenance_bot removed a project: Patch-For-Review.Oct 10 2023, 5:30 AM

ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.1; 2023-10-17).Oct 10 2023, 6:00 AM

MusikAnimal moved this task from Review/Feedback 💬 to QA 🐛 on the Community-Tech (CommTech-Kanban) board.Oct 10 2023, 4:33 PM

Change 964597 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/core@REL1_41] diffs: add line number headings to inline diffs

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

gerritbot added a project: Patch-For-Review.Oct 10 2023, 4:35 PM

MusikAnimal updated the task description. (Show Details)Oct 10 2023, 4:47 PM

Change 964599 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/core@wmf/1.41.0-wmf.30] diffs: add line number headings to inline diffs

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

Change 964597 merged by jenkins-bot:

[mediawiki/core@REL1_41] diffs: add line number headings to inline diffs

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

ReleaseTaggerBot added a project: MW-1.41-notes.Oct 10 2023, 6:00 PM

Change 964599 merged by jenkins-bot:

[mediawiki/core@wmf/1.41.0-wmf.30] diffs: add line number headings to inline diffs

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

Mentioned in SAL (#wikimedia-operations) [2023-10-10T19:49:00Z] <hmonroy@deploy2002> Started scap: Backport for [[gerrit:964599|diffs: add line number headings to inline diffs (T346460)]]

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.30; 2023-10-10); removed MW-1.41-notes.Oct 10 2023, 8:00 PM

Mentioned in SAL (#wikimedia-operations) [2023-10-10T20:07:24Z] <hmonroy@deploy2002> musikanimal and hmonroy: Backport for [[gerrit:964599|diffs: add line number headings to inline diffs (T346460)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Maintenance_bot removed a project: Patch-For-Review.Oct 10 2023, 8:15 PM

Mentioned in SAL (#wikimedia-operations) [2023-10-10T20:19:26Z] <hmonroy@deploy2002> Finished scap: Backport for [[gerrit:964599|diffs: add line number headings to inline diffs (T346460)]] (duration: 30m 26s)

@MusikAnimal Everything looks good on the AC's so far and just waiting for more information on AC4. Please keep me posted when you find out, thanks!

Status: ✅ PASS
Environment: Beta: 1.42.0-alpha (625f208)
OS: macOS Sonoma 14.0
Browser: Chrome 117, Firefox 118, Safari 17.0
Device: MBA M2
Emulated Device:: n/a
Test Links:
https://en.wikipedia.beta.wmflabs.org/w/index.php?title=World_of_Warcraft&diff=603553&oldid=603489

✅AC1: Inline diffs should surface line numbers as provided by wikidiff2

2 Column	Inline

✅AC2: The line numbers should be in the form of Line 5 ⟶ 10:, where 5 is the previous line number, and 10 is the line as of the new revision.

2 Column	Inline

✅AC3: If the line numbers haven't changed (i.e. both the previous and new line numbers are 5), it should use the shorter message Line 5:

2 Column	Inline

✅AC4: If the reducedLineNumbers option is true (as it is on our cluster), you shouldn't ever see Line 1:, but you may see for example Line 1 ⟶ 5. It's only for Extension:Translate. Test case reducedLineNumbers is in gerrit.

Hebrew