Improve diff message "no files"

[Emmanuel Blot]

When a changeset contains only minor modifications (blank line, whitespace changes …) and the matching "ignore" checkboxes are selected, Trac outputs a confusing message:

(No files)

whereas the actual message should be something like "(No visible changes)", as the changeset does contain files, but the selected diff options lead to an empty diff.

This is confusing especially as Trac conveniently keeps track of the user diff preferences from one diff to another - which implies that a user can get a "(No files)" message as soon as he wants a diff.

[sid]

Simple patch to fix this issue:

trac/versioncontrol/templates/changeset.html

@@ -135 +135 @@
           <dt class="property location">Location:</dt>
           <dd class="searchable"><a href="${req.href.browser(location, rev=new_rev)}">$location</a></dd>
         </py:if>
-        <dt class="property files">${files and 'Files:' or '(No files)'}</dt>
+        <dt class="property files">${files and 'Files:' or '(No visible changes)'}</dt>
         <dd class="files">
           <div class="legend" id="file-legend" py:if="filestats">
             <dl>

[osimons]

Any reason why this cannot just be committed? The new text provides good information, both for whitespace and hidden properties that can make a changset appear completely empty.

[Christian Boos]

That change is fine for me, we can commit it.

[osimons]

Actually, that did not turn out so nice with regards to formatting. There is only room for 1-2 words, so it breaks a line and just looks a bit out of place on the side of the page.

Alternative proposal:

trac/versioncontrol/templates/changeset.html

@@ -135 +135 @@
           <dt class="property location">Location:</dt>
           <dd class="searchable"><a href="${req.href.browser(location, rev=new_rev)}">$location</a></dd>
         </py:if>
-        <dt class="property files">${files and 'Files:' or '(No files)'}</dt>
+        <dt class="property files">Files:</dt>
         <dd class="files">
+          <py:if test="not files">No files with visible changes.</py:if>
           <div class="legend" id="file-legend" py:if="filestats">
             <dl>
               <py:if test="filestats['add']"><dt class="add"></dt><dd>${filestats['add']} added</dd></py:if>

I basically renders:

Files: No files with visible changes.

That will also give better space for future l10n messages that might need more room.

Only question: Any reason why it should not render correctly being placed inside the <dd> tag? Any circumstance where it will not be alone, or why we can't have just text instead of the ususal lists?

[Christian Boos]

That latest change is better. Still, I think that for "perfect" clarity we should make clear whether the list of files is empty because there's no files in the changeset or no visible change according to the current diff options.

I'm OK for committing the intermediate improvement of comment:5, but maybe leave the ticket open for 0.11.1 as a reminder to implement the above.

[osimons]

When testing this, I found that even property changes rendered as a file. I was not able to think of anything (using Subversion) that could make a change without listing some file - ie. in all circumstances it was diff options that made this list appear.

Perhaps with other versioncontrol systems? Could there be circumstances with full diff that as change is made that has no file references? The versioncontrol code is really not what I know best…

[Christian Boos]

Well, the situations I had in mind for the no files were admittedly corner cases.

The first was when a property change is hidden on purpose ([browser] hide_properties), e.g. r5705.

The second situation was root property change. But in this case we generate a pseudo-"(root)" file entry.

So the only confusing situation would be r5705, as there's no way a change of diff options will make that prop change visible.

I'll attach a patch shortly that would instead render this minimal information about the hidden property change:

• i18n

  • Property svnmerge-integrated changed

That will give some hint about what's going on in changesets like r5705, while preserving the uncluttered view that hide_properties was meant to provide.

[Christian Boos]

Show at least Property xyz changed for a change of a hidden property, so that there's at least one file entry in the Files: list.

[Christian Boos]

Ah, and of course, besides the above corner cases that are now addressed, there's one more situation to think about: changesets restricted on a path outside of the scope of the changeset, e.g. [6334/sandbox]. In this case, it's a legitimate (No files).

[anonymous]

are there also changesets that could be empty due to permission settings, ie, you don't have permission to see the changed files?

[osimons]

Replying to anonymous:

are there also changesets that could be empty due to permission settings, ie, you don't have permission to see the changed files?

Yes. Both regular permissions, and I Just configured a project today myself where anonymous has CHANGESET_VIEW for seeing changesets in Timeline and reading log messages, but no FILE_VEW permissions to see the changed files. That will actually list (No files.) for all changesets.

[Christian Boos]

This issue is less trivial than it seemed at first. I think we should keep the (No files) for now and improve that message once with the correct information later on:

• "(No files)" when this is effectively a changeset without files (i.e. property changes only)
• "(No files in the restricted scope)" when there are files in the changeset but outside of the restricted path
• "(No visible changes)" when there should be files but there's no changes to show because of the current diff options
• more?

[Emmanuel Blot]

Replying to cboos:

This issue is less trivial than it seemed at first. I think we should keep the (No files) for now and improve that message once with the correct information later on:

The current implementation is unsettling for the end user.
If a changeset only contains property change(s):

• the diff view shows "(No files)" - and nothing more, while
• the changeset view shows the actual property change (with the property name and value)

[Christian Boos]

Removing keyword, as there's no patch currently meeting the requirements.

[Ryan J Ollos]

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

[Ryan J Ollos]

Milestone renamed