Show the difference between edited statuses

[nikclayton]

Diff each status against the previous version, comparing the different HTML as XML to produce a structured diff.

Mark new content with <tusky-ins>, deleted content with <tusky-del>.

Convert these to styled spans in ViewEditsAdapter.

Fixes #3306

[nikclayton]

Looks like this:

[sl1txdvd]

I tried this and I love it! Works very well and helps a lot.

[sl1txdvd]

There seems to be a problem with German umlaut. The diff usually shows changes on a whole word basis, but when I use words with "ä" I get really hard to parse diffs that keep the "ä" in place and arrange the changes around it. When I substitute "ä" with "ae" this is not happening.

[nikclayton]

Hmm.

ViewEditsViewModel contains this:

                            loader.config = DiffConfig(
                                false,
                                WhiteSpaceProcessing.PRESERVE,
                                TextGranularity.SPACE_WORD
                            )

Could you try different builds with different values for the last parameter, and see if that has any effect?

Valid values are in https://github.com/pageseeder/diffx/blob/master/src/main/java/org/pageseeder/diffx/load/text/TokenizerFactory.java

I'll take a look at their code separately and see if there's an easy fix.

[nikclayton]

It's probably https://github.com/pageseeder/diffx/blob/master/src/main/java/org/pageseeder/diffx/load/text/TokenizerBySpaceWord.java#L67.

[nikclayton]

And if you add a link to this issue to a status that shows the problem that would be great.

[sl1txdvd]

It's probably https://github.com/pageseeder/diffx/blob/master/src/main/java/org/pageseeder/diffx/load/text/TokenizerBySpaceWord.java#L67.

Yes, that regex is too strict. I tried with a local diffx branch where I changed the [A-Z][a-z] parts to \p{L}\p{M} and that seems to work better. I'll try some more tomorrow.

Before I tried all standard options and while some technically worked, they were not very elegant. Basically they'd just display the whole text of both posts even if you replace just one word.

[sl1txdvd]

And if you add a link to this issue to a status that shows the problem that would be great.

My test post is a DM. I'll do one that I can link here tomorrow.

[sl1txdvd]

Link to post showing the problem
https://uff.rip/@rauschen/109867330918031758

The adjusted diffx seems to fix it. Here some partial screenshots with a Russian example.

What it looks like now:

diffx changed:

[sl1txdvd]

Here is what I did to diffx: https://github.com/sl1txdvd/diffx/tree/unicode

[nikclayton]

Thanks for that, very helpful.

Looking at the code it seems like there's no way to specify a custom tokenizer using the current API:

SAXLoader expects a DiffConfig, and uses that to get a tokenizer using TokenizerFactory. But there's no way to give SAXLoader a custom tokenizer, and all the classes are final, so they need to be completely re-implemented.

I'll file an issue with the project. If they fix it quickly, great. If not, we can fork it and fix it.

[nikclayton]

Here is what I did to diffx: https://github.com/sl1txdvd/diffx/tree/unicode

Upstream have taken that and fixed it in 1.1.1, which this now uses. The edits to https://uff.rip/@rauschen/109867330918031758 now display correctly.

[sl1txdvd]

Awesome! One of my favourite new features. I use it a lot and see more people asking about it.

[nikclayton]

This is probably the right set of abstractions in the code now, so the last bit is probably to figure out how we want to display the differences.

Here's a screenshot with the current code.

Because the insert/delete markers are Android spans we can do anything, within reason.

The current code demonstrates setting background colours, bold, strikethrough, text alpha -- that's probably a bit much. I suspect the choice is either:

1. Background colours (red for deleted, green for inserted)
2. Strikethrough (possibly with lower alpha) for deleted, bold for inserted

Does anyone have strong opinions either way?

The one final feature I think we might want to include before this merges is a menu item to toggle the display of differences off and on. Although, if the user is looking at this screen already there's a good argument that they're here because they want to easily see the differences, so this wouldn't be something that gets used.

And if we do use background colours I'll add a bit of inset and padding so it's not the harsh square corners that are currently in the screenshot.

[sl1txdvd]

Does anyone have strong opinions either way?

For me it's definitely the red/green. But it won't work great for people with colour blindness I suppose? Maybe a switch for one or the other would be best.

Edit: I was referring to the old colours as seen in my screen shots.

[connyduck]

Fancy 🧐

Almost unreadable on dark theme though

[nikclayton]

Latest screenshot, with rounded corners:

[nikclayton]

Does anyone have strong opinions either way?

For me it's definitely the red/green. But it won't work great for people with colour blindness I suppose? Maybe a switch for one or the other would be best.

I checked screenshots with the colours against https://www.color-blindness.com/coblis-color-blindness-simulator/.

Light mode appears fine, the colours are distinct in every variation it tests for.

Dark mode is tougher to distinguish with Red-Blind/Protanopia or Green-Blind/Deuteranopia.

I'd be OK with a preference with switches for:

• Use background colours
• Strikethrough deleted text
• Bold inserted text

(so it's three preferences under the hood).

It can pop up a dialog with the three switches and an example view that updates as they change them to show the difference.

[sl1txdvd]

FWIW the text alpha setting makes it very hard to read in dark mode in combination with background colour.

Also the alpha is different where text and strikethrough overlap, making these parts appear lighter/ less transparent than the rest. That's a bit odd.

[connyduck]

Yes, should be a solid color to avoid this

[nikclayton]

This needs to be rolled back, I've discovered an unintended bug. The span type doesn't work over multiple lines, so if the text exceeds the screen margin it extends off the end.

I've got a fix which I'm testing, although the UI isn't quite as pretty.

[connyduck]

That is not bad enough for a rollback