Actually, in case of initial file checkins, having “added files” (and “removed file”) hidden by default (independently of the preferred diff settings) would be great.

This way we could set our preferred diff mode and initial file checkins with a lot of added files would not be as heavy to load.

Anyway I think it’s not so useful to look at diff for a new file. We may want to look at the file content but in this case it’s easy to unhide one file at a time.

By appending ?diff=0&udc=1 to any /info page, the preferred-diff-type is overridden to 0 for the lifetime of the fossil_display_settings cookie (see /cookies). Maybe this is some relief.

(This may not work on fossil-scm.org itself due to harsh anti-bot measures.)

Exposing the fossil diff --verbose option to the web UI sounds like a good idea.

But on the other hand, a more general approach to deal with large diffs in the web UI may be preferred, because new and deleted files are not the only (but likely the most common) case where diff generation can time out. And diffs of individual files can be hidden by check-box (I can even do this by keyboard shortcuts).

On latest commit on trunk (8b73fbbd1a860708): diff rendering is now skipped for Added and Deleted files (amongst other diff related noise reduction).

Content of the file is accessible via link in this case.

I think this will help your use case for your "initial file check-in".

I need to find out whether or not I like this. As I tend to work with files and projects of sizes manageable for humans, I'm not usually running into diff timeouts, and I find this change quite invasive.

But it doesn't affect raw patch output (/vpatch), as far as I tested it, or does it? Because that would really break my Fossil workflows!

The change will only avoid to process and render a diff when there is no really a diff to process. (and in same time, remove warnings that become pointless like that binary file diff could not be processed).

I don’t understand what is invasive about it.

By "invasive" I mean that the patch fundamentally changes the output and information content of the web UI diffs from what I'm used to and working with every day.

Have a look at:

• https://fossil-scm.org/home/info/cfdd666533

With your patch, the contents of the added file "src/fossil.info-diff.js" are missing, whereas previously it was displayed in its entirety.

I'm doing my code reviews with the Fossil web UI diffs, and your change disrupts my workflow, as I can no longer overview ALL the changes of a certain check-in on a single page.

So I'll have to revert this for my local Fossil build, but I hope we can also back-out this upstream.

I can no longer overview ALL the changes of a certain check-in on a single page.

That's an intentional feature of this change. It's not especially useful to speak of "diffs" when one side of the difference is missing or 0 bytes long.

The full text of the added file is one click away.

It's not especially useful to speak of "diffs" when one side of the difference is missing or 0 bytes long.

I beg to differ, but it's subjective, of course.

The full text of the added file is one click away.

Still disruptive and breaking years of habits! :(

I'm admittedly ambivalent about the change too. If this was before fnc
I would likely object but I no longer do many, if any, code reviews in
the browser such that it doesn't disrupt my workflow.

Nonetheless, my "ideal" solution would be to provide a knob to toggle
whether added and deleted files are emitted or elided. Perhaps a new
diff-verbose versionable option and a switch on the web ui akin to
Hide Diffs or Sync side-by-side scrolling.

That said, that is not a blocking requirement from my perspective and I
appreciate that mgagnon@ made the abridged ui smart enough to emit diffs
of renamed files that contain changes, and that /vpatch still produces a
complete diff. I'm also sympathetic to how this disrupts your workflow,
Florian, as it is a significant change to the UX. Perhaps implementing a
new setting to toggle this behaviour is a workable solution for you?

I'm fine with a local patch, because I think Fossil already has too many options -- knobs and switches everywhere!

However, it's not clear to me why this feature wasn't planned as an option from the beginning. Because the changes to the information content of the web UI diffs are substantial -- unlike for the diff-word-wrap branch, which only affects the presentation and not the information content, but for which the consensus was explicit that this would only be accepted as an option.

The branch name (diff-web-noise-reduction) and check-in comments don't give any hints that added files are no longer shown. So I wonder when this feature was discussed in the chat (which I can't follow due to time and timezone constraints, unfortunately) whether all the involved decision makers were aware of this fundamental change, or whether this was simply overlooked.

Anyway, as already mentioned in the beginning, I prefer a local patch over yet another option in Fossil.

However, it's not clear to me why this feature wasn't planned as an option from the beginning.

i suspect that this one falls under the category of "we didn't really know what we want until we saw it." After many years of looking at whole-file diffs for new files, they do feel rather unnecessary and, as Warren points out, the whole content is one click away if we really need it.

So I wonder when this feature was discussed in the chat ...

Martin brought it up in /chat right after his commit. Both Larry and myself voiced support for this change, and we heard no objections, so it was (after an okay from Richard) merged about a day and a half after the initial commit.

Sidebar regarding making it an option: it would have to be a server-side option because that's where the diffs are rendered. The down-side to a server-side option for this is that it would apply to all users, so it would always represent the preference of the administrator rather than each user. The side-by-side scrolling option, in contrast, is 100% client-side.

Or maybe a query parameter passed to the server representing the client-side state, similar to the options held in the display cookie?

But I'm completely fine if merging this change was a deliberate decision. And I wouldn't be eager for yet more options, either.

I'm sorry if this change affected you negatively guys. (Mark and Florian).

Just out of curiosity, when you do "code review", you read every single lines
of deleted files too ?

As for added files, I can understand you may want to read it, but since there's
not really a diff to see and reviewing brand new code is not something that take
a few second, the extra click away to have a better view of the new code
doesn't seems disruptive to me. But I agree with Florian, this kind of thing
may be subjective.

In the other hand, when I want to concentrate on actual diff to review, the added
and deleted files are really on the way when looking at the really nice fossil
side by side diff view which is good at highlighting only differences and making
the diff as compact as possible.

Perhaps a kind of "verbose diff" link would give you a single click to make all
those added/deleted files diffs re-appear and would be an improvement for
your workflow ? Kind of middle ground between nothing and a new setting.
(a bit like the " Ignore Whitespace" link ?)

No need to apologise! You rightly proposed it in /chat and sent it to a
branch first. If I really didn't like it, I would've objected in /chat.
Besides which drh@ ok'd it and that's all that matters at the end of the
day :)

I only chimed in to see if there was a less disruptive option for
Florian as I can sympathise with his position. And I know he doesn't
frequent /chat as much as us.

To answer your question, I don't often have deleted files, more renames
and added files but when I do, I don't think I ever read every removed
line. I'll usually scan to verify the removal matches what I expect. For
me, it's more important the actual diff that might be used in the
patch -p0 < p.diff contains all changes and you haven't impacted that.
However, I think it makes more sense to show all changes faithfully
by default, and provide a means to silence ouput from there. But like I
said, it's not a blocking preference at this point. And thanks for
considering potential improvements that might satisfy all positions :)

I definitely agree with the option overload!

It often makes me think about Spolsky's thoughtful article^. In any
case, as long as you're happy to carry another local patch I'm happy
with Martin's change.

https://www.joelonsoftware.com/2000/04/12/choices/

I'm sorry if this change affected you negatively guys. (Mark and Florian).

No worries! But since the feature to hide added files from the diffs was not advertised in the check-in comments, I missed it in my first quick tests, and wanted to make sure the change was intentional and also noticed by the deciders.

Just out of curiosity, when you do "code review", you read every single lines of deleted files too ?

Sometimes I do, and sometimes I don't. But I want to see what was / will be added, and I can still scroll over new content that was already reviewed separately, which is not as disruptive as reloading the page, or opening another page.

Perhaps a kind of "verbose diff" link would give you a single click to make all those added/deleted files diffs re-appear and would be an improvement for your workflow ? Kind of middle ground between nothing and a new setting. (a bit like the " Ignore Whitespace" link ?)

Then I will have to maintain a local patch to eliminate that one extra click and use the "verbose diff" link by default, so it doesn't matter to me. I've been using the web UI diff view for years, and I really want it unchanged.

The only situation where an option or separate link would do me any good would be if it could be set as the default for fossil-scm.org, even with short-lived browser cookies. But with the new anti-bot defenses, URLs with extra query parameters are blocked, so a ?verbose=1 query parameter wouldn't help me.

And another link would add even more clutter to the UI.

It's fine for me now that I know this is what the Fossil people want.

For me, it's more important the actual diff that might be used in the patch -p0 < p.diff contains all changes ...

This is also essential for me!

For me this completely breaks using the web UI to review a commit. When I scroll through a commit, the "Modified" lines get filtered as noise by my brain so I don't see them. However, now there are "Added" lines that also get filtered. It has happened several times now already that I missed added files during a review. For a commit with many small added files, it gets even worse. What used to be a few pages of scrolling are now 20 or 30 clicks for files that are all one line, making the web UI entirely unusable for such commits.

IMO this is a breaking change, and should really be an option at the very least, probably even default disabled (though I don't care about the default too much, but changes of defaults that break users are never good).