Fossil Forum

How to set unified diff by default?
Login

How to set unified diff by default?

(1) By Julcar (julcar) on 2021-03-01 06:28:57 [link] [source]

On commit view, the default diff view is side-by-side, but on phones is a pain to read them, specially on vertical mode, but that problem solves with unified diff, so, how can I set it as default?

Thanks.

(2) By Stephan Beal (stephan) on 2021-03-01 06:48:22 in reply to 1 [link] [source]

...unified diff, so, how can I set it as default?

i don't recall there being a setting for that, and the server doesn't have enough information to determine your device type. We could add a setting but it would be repo-specific, so would do you absolutely no good on foreign repos and would likely annoy the majority of users, who are on desktop-grade devices.

My recommendation is that we instead remember the diff style choice in the "display preferences" cookie, so that it's browser/device-specific.

(3) By Warren Young (wyoung) on 2021-03-01 07:55:33 in reply to 2 [link] [source]

+1

(4.2) By Stephan Beal (stephan) on 2021-03-01 18:07:45 edited from 4.1 in reply to 2 [link] [source]

My recommendation is that we instead remember the diff style choice in the "display preferences" cookie, so that it's browser/device-specific.

It turns out that we already do that but it only takes effect if the "udc" URL parameter is included. Fossil's internals try aggressively not to emit changes to that cookie unless needed, and one of its prerequisites is that "udc" must be set. That seems a bit harsh to me, but Richard certainly had his reasons for doing so.

One option would be to add the udc parameter to the various diff format links, but then we'd end up with links which people would copy and include the udc part, which would cause users visiting such links to toggle their display preference without realizing it. IIRC, there was some effort in the past year or so to remove those flags from links to avoid exactly that problem. (Edit: i've found no proof of that, so maybe that wasn't a real thing.)

So that option is out.

My next preference would be to simply remove the requirement for udc. The cookie-management code already recognizes when the cookie changes and refuses to emit it if it has not changed, so requiring udc in addition to that seems unnecessary to me. This change would require modifying only half a line of code:

fossil:/info?name=c6985657001a9a6&ln=182

Suggestions?

(5) By Julcar (julcar) on 2021-03-01 16:21:21 in reply to 2 [link] [source]

Of course I was meant to repo-specific settings, I just want my repo(s) to have unified diff by default

(6) By Stephan Beal (stephan) on 2021-03-01 16:43:27 in reply to 5 [link] [source]

I just want my repo(s) to have unified diff by defau

i think there's a reason we haven't done that already, as described in my initial response. Such a setting would be extremely user-unfriendly for the majority of users. Unified diffs are, by and large, more difficult to read than side-by-side diffs, except on narrow screens (which are not our primary UI target - this is a software development tool, not a social media app) or on files with really long lines (e.g. markdown files edited via the /fileedit page).

If we can convince Richard to let us do away with the "udc" (User Display (preferences) Cookie) URL parameter check which currently makes it onerous for a user to update that cookie, we can solve this in a much more flexible way: you can have unified on your phone and sbs on your desktop.

Ideally, IMO, the page should simply remember the last format you explicitly clicked and default to that for the lifetime of the cookie, with no extra user interaction or URL editing required. Then we'd have the "sensible default" for the majority-case users (desktop) and an easy way for narrow screens to switch or for people to turn off diffs altogether on low-bandwidth or low-memory devices.

(7) By John Rouillard (rouilj) on 2021-03-01 17:59:15 in reply to 4.1 [link] [source]

It turns out that we already do that but it only takes effect if the "udc" URL parameter is included in the page.

Thank you. I have been trying to figure out why the diff state isn't being saved and applied. The code I inspected looked like it was saved. I missed the udc requirement. I used to have it working but it stopped at some point. I made an attempt to bisect it without luck.

Now at least I know I am not crazy.

(8) By Julcar (julcar) on 2021-03-01 19:53:34 in reply to 6 [link] [source]

My though is, that a "default-diff-style" setting would be enough, anyway the diff style buttons won't disappear from the diff page, just one of them is applied by default, in this case, the choice of the repo's owner/mantainer (take in mind the fact that very often a single developer contributes to the repo, as in my case).

(9) By Richard Hipp (drh) on 2021-03-01 20:40:54 in reply to 1 [link] [source]

Now on trunk:

  • If the diff type (unified vs. side-by-side) is not specified by a query parameter or by the user display preferences cookie, then look for the "preferred-diff-type" setting. If that setting exists and is greater than zero, then use it. If there is no such setting or the setting is less then or equal to zero then look at the User-Agent string in the HTTP header, and if the User-Agent string seems like a mobile device, then use unified, else use side-by-side.

(10.1) By Julcar (julcar) on 2021-03-01 20:56:05 edited from 10.0 in reply to 9 [link] [source]

Wow, very thanks Richard!

(11.1) By Julcar (julcar) on 2021-03-01 20:55:51 edited from 11.0 in reply to 9 [link] [source]

Deleted

(12) By Marcelo Huerta (richieadler) on 2021-03-01 22:04:50 in reply to 9 [link] [source]

Now on trunk:

  • If the diff type (unified vs. side-by-side) is not specified by a query parameter or by the user display preferences cookie, then look for the "preferred-diff-type" setting. If that setting exists and is greater than zero, then use it. If there is no such setting or the setting is less then or equal to zero then look at the User-Agent string in the HTTP header, and if the User-Agent string seems like a mobile device, then use unified, else use side-by-side.

Setting preferred-diff-type globally appears not to have an effect; is this intended?

(13) By Richard Hipp (drh) on 2021-03-01 22:19:33 in reply to 12 [link] [source]

No. I don’t know why that does work. I‘ll investigate when I get back to the office

(14.1) By Richard Hipp (drh) on 2021-03-01 23:19:59 edited from 14.0 in reply to 12 [link] [source]

The reason is that the ~/.fossil file (or ~/.config/fossil.db file) that holds the global settings is not attached or read by web-pages. That's because the ~/.fossil file is located relative to the users home directory, but CGI scripts an other dynamic page generates often lack the concept of a home directory. So global settings do not affect web pages, only CLI commands.

In the case of Fossil's own self-hosting repository, the web pages are generated by a process that is running inside of a chroot jail. So they could not access the home directory of the user even if they wanted to and knew where that home directory was. I suspect that is a relatively common scenario.

I don't think we want to start having web pages access a common configuration database.

(15.2) By Florian Balmer (florian.balmer) on 2021-03-19 13:50:20 edited from 15.1 in reply to 9 [source]

There seems to be problem with the display cookie "diff" setting.

Steps to reproduce with the fossil-scm.org website:

  1. Open the Timeline.

  2. In another window, or tab, open the Cookies page.

  3. Press "Delete" on the Cookies page to clear the display cookie.

  4. On the Timeline page, click the link of any check-in, and note the diff mode looks like "Side-by-Side Diffs".

  5. Refresh the Cookies page, and examine the "diff" cookie setting, which looks like diff: "" (empty string; on my local tests on Windows, the string looks like random uninitialized data).

  6. On the check-in page opened from the Timeline, navigate to another check-in, and note the diff mode now looks like "Hide Diffs". If the "diff" cookie setting is uninitialized or empty, the diff mode should not change.

Disclaimer: only tested with the Chromium web browser.

Bisecting results:

So the problem seems to have been introduced with the skin-preference-cookie branch.

Also, I'm not sure if the Last Good version works as intended, as the "diff" cookie setting is not updated if another diff mode is selected, i.e. when navigating to a new check-in, the diff mode always looks like "Side-by-Side Diffs" (or whatever has been specified for the preferred-diff-type setting). But note that this problem has been resolved in the First Bad version.

Edit: Fixed link. Clarified that the second problem found in the Last Good version was resolved in the First Bad version.

Edit 2: This time fixed the correct link.

(16) By Stephan Beal (stephan) on 2021-03-19 14:38:29 in reply to 15.2 [link] [source]

Refresh the Cookies page, and examine the "diff" cookie setting, which looks like diff: "" (empty string; on my local tests on Windows, the string looks like random uninitialized data).

i will take a look at that this evening. Thank you for the detailed observations.

(17) By Stephan Beal (stephan) on 2021-03-19 16:06:18 in reply to 15.2 [link] [source]

(empty string; on my local tests on Windows, the string looks like random uninitialized data).

Fixed, but i got really lucky in finding it. Your note about uninitialized data was the only reason it was resolved so quickly:

fossil:e378f9300e9da364

Details: the routine which sets the default preferred cookie value passed a non-static function-local char array to cookie_link_parameter(), but cookie_link_parameter() does not copy its input strings - it expects them to be static or malloced-and-never-freed memory. After the diff-pref function returned, the content of those bytes was undefined. So... adding static to the function-local char array solved it.

Thanks again for the detailed report!

(18) By Florian Balmer (florian.balmer) on 2021-03-21 13:22:47 in reply to 17 [link] [source]

Wow, nice catch, thanks! This explains my confusion with the Last Good version being broken in a subtly different way -- it all depends on what is stored at the location of the local zDflt variable when it's accessed from outside of its valid stack frame. So I decided to write down my findings in detail, at least, hoping this might clear up my confusion.