Re: [RFC/PATCH 0/7] Tab updates

[Kai <kai willadsen gmail com>]

On 24 September 2010 16:00, Peter Tyser <ptyser gmail com> wrote:
>> Here's a strawman proposal. We should omit repetition of file and
>> directory names, shorten path names down to the closest unique
>> element, and generally abbreviate where it makes sense. For some
>> specific examples, a VcView of /home/me/Geekery/meld should probably
>> display as:
>>    [git] ~/Geekery/meld
>> and if the path component got too long, we'd just abbreviate it to:
>>    [git] meld
>> and leave the detail to the tooltip. FileDiff tabs launched from a
>> VcView should be roughly the same, so:
>>    [git] meld/meld/filediff.py
>> or
>>    [git meld] meld/filediff.py

So this completely fell off my radar. My apologies for that. I still
like your suggestions though, so here goes...

I've written a whole lot of stuff below. The short version is:

I can't really tell which of these changes will work and which won't
without using the results. There are some obvious fixes we can do
(i.e., don't repeat identical filenames in FileDiff tabs, don't show
tmp directory stuff in VcView file comparisons, etc.). I'd like to get
some of these done, and then figure out where to go from there.

Also, patches 2, 3 and 7 from this series have been merged.

> I don't personally see much advantage in including the "[git]" prefix
> in the tab label for VcViews.  If someone runs "meld ./", they are
> explicitly stating that they want to do a VcView diff, thus the
> "[git]" prefix doesn't add much useful info to them and takes up
> horizontal screen space.  If a directory is under multiple revision
> tools (eg svn and git), the user can use the VC dropdown menu on the
> right to either see or change which tool is currently being used for
> diffs.

There are several functions for the prefix. First, it tells you that
it's a VC comparison. Yes, the icon does this too, but relying on
icons is not a great idea. Then there's being able to tell which VC
you're in from the label; as you say you can do this from the VC
dropdown too. Finally, it lets you differentiate between same-name
files in different repos. e.g., looking at file "foo.c" in the "bar"
repo and the "wibble" repo.

I could go either way on this. I'd like to test both options out for a
while and see whether the additional prefix is worthwhile information
or is, as you suggest, just noise.

> As far as shortening the path shown in the VcView label to something
> logical like your examples of "~/Geekery/meld", that would be nice.
> My issues with this method are:
> 1. It seems somewhat complicated to do cleanly and intelligently.  eg
> how can we intelligently shorten the name?  Is there a way to cleanly
> convert /home/me/Geekery/meld to ~/Geekery/meld?

I'd guess that we could check for path.startswith(os.path.expanduser("~"))

> How long is "too long" for the label?
> Does "too long" depend on font size and the static tab width?

Ah - the hard questions. Tab width is currently set to be the
approximate width of 30 characters in tab label font size, so (30 - VC
prefix - last component of path). Obviously, part of the goal here is
to fit more tabs on screen, so that 30 might drop.

> Can this shortening code be shared with the same
> code to shorten DirDiff and FileDiff paths?

Probably. That code needs to be cleaned up anyway, iirc.

> 2. A user explicitly starts meld to diff a specific directory, eg
> "meld ./", or "meld ~/Geekery/meld".  Does including the path in the
> tab label provide much benefit?

In some cases, no. But how many directory comparison tabs do you
really have open at one time? It would be rare for me to have more
than one, and very rare to have more than two.

> Additionally, the full path is shown
> in both the tooltip (assuming these patches are merged in some form)
> as well as the dropdown directory selector right beneath the VcView
> tab.

I merged the tooltip-setting code for VcView, but missed the DirDiff
one. I think it was tied up with all of the other changes?

> 3. If the path is only sometimes shortened, I'd think that would be
> somewhat confusing.  eg "meld /home/juser/meld" shows ~/meld, "meld
> /usr/meld" shows [/usr/meld], but "meld /mnt/home/juser/meld" shows
> just [meld] because its too long.

Good point. If we shortened in those kinds of cases, we'd need to add
ellipses to make it clear what had been done.

> So I like the idea, but personally wonder if the benefit of
> intelligently shortening directory paths is worth the added code and
> UI complexity.
>
>> also abbreviating the path component if required. DirDiff tabs (and
>> FileDiff tabs launched from DirDiff) are more complicated. I'd say
>> that we want to trim identical prefixes and suffixes from both paths
>> for the tab label, so comparing /home/me/Geekery/meld/meld/ui/ to
>> /home/me/Geekery/meld-1.3.3/meld/ui should yield something like:
>>    [meld -> meld-1.3.3] ui/
>> and comparing meld/filediff.py from that DirDiff would give:
>>    [meld -> meld-1.3.3] filediff.py
>> It's possible that just taking the first differing element in the path
>> would suffice here.
>
> I can see the benefit of the method you describe, but for my usage
> scenarios would prefer a simpler scheme for the following reasons:
> 1. For both the DirDiff and FileDiff, the directories or files being
> diffed are shown in the dropdown file selector immediately below the
> tabs.  So if I'm actually on the DirDiff or FileDiff tab, its very
> clear what is being diffed and I don't need to look at the tab label.
> 2. If I'm not on the DirDiff or FileDiff tab and I'm curious about
> what the tab contains, hovering on it should explicitly show what is
> being diffed (assuming this patch is merged in some form)

The problem with this is that it clean lead to scrubbing over tabs
looking for what you wanted. Having the tooltips there for full
information is great, but I don't want people to actually have to use
them if it can be avoided.

> 3. If the directories being diffed have long names, the tab length
> gets very long, eg for a linux kernel diff between 2 versions:
> "[linux-2.6.30-rc5 -> linux-2.6.30-rc6] Makefile".  Every tab would
> contain the same [linux-2.6.30-rc5 -> linux-2.6.30-rc6] which seems
> redundant and a waste of horizontal space.

You have a point. This is where I wonder whether we shouldn't
implement and enforce a one window = one 'top-level' (i.e., dirdiff or
vcview) mode. Most issues with tab labels just go away at that point.

>> Anyway, that's my strawman, so... comments?
>
> For me, I personally like a simple, short, consistent tab naming
> scheme..  If I only have a few tabs open, its pretty easy to keep them
> straight, so the added tab label text doesn't provide much benefit for
> me.  If I have a ton of tabs open, I'd like to fit as many of them as
> possible on my screen (ie they'd have short labels) so I can switch
> between them quickly.  That was one of the main motivations of this
> patchset - I can currently only fit 4-5 FileDiff tabs open at a given
> time, so navigating between them is somewhat difficult which is
> annoying when diffing a change that affects many files.

This may or may not help, but we should possibly also have a "Windows"
or "Documents" ("Comparisons"?) menu that lists open tabs. That at
least would provide a single place to see all current tabs whether
they were off screen or not.

> That being said, its just my personal preference and others usage
> scenarios may differ greatly.
>
>>>  Update NotebookLabel tab drawing
>>
>> You're correct that the HIG recommends dynamic tab widths, but I
>> suspect that this might an out-of-date recommendation; FWIW, Meld's
>> tab drawing was heavily inspired by Epiphany, and it's difficult to
>> think of a more Gnome-y app. It would be interesting to know what HIG
>> version 3 is going to say here, though with natural text width in 3.0,
>> dynamic tab widths + ellipsizing should make more sense too.
>
> I used gedit as an example of dynamic tab widths.  Most any Gnome
> preference dialog I see uses dynamic tabs for what its worth.  But, it
> looks like a few heavy hitters don't, eg gnome-terminal and nautilus.

Right - a majority of gnome-y applications tend not to follow this
guideline, or at least not consistently. Having said that, I do think
that gedit is a pretty good app to look to for UI, considering the
likely crossover in user base.

For me, the real problem with dynamic tab widths is that our potential
tab width variability is (at least) three times greater than other
apps. In gedit, if you edit a 6 character file alongside a 10
character file, there's 4 characters difference. If you do a three-way
merge with 6 character files alongside 10 character files, there's ~12
characters difference between them! And that's before you start
dealing with showing prefixes.

As below, I don't know what our options for tab sizing are, but I
maybe dynamic tab widths would be acceptable with some fairly strict
min/max sizes. I'm thinking something like min 15, max 30 chars.

> As an alternative to using this patch's dynamic tab width, what about
> using dynamic tabs that fill the entire horizontal space?  eg 2 tabs =
> 50% width each, 3 tabs = 33% width each, 4 = 25, etc.  gnome-terminal
> uses this scheme, along with other common apps like firefox and chrome
> (not exactly Gnome apps though...).

I haven't looked into the options for tab sizing, and I suspect that
we'll hit GTK limitations, but...
Firefox has a maximum tab size and a minimum tab size. It will show
tabs at their maximum (constant) width with ellipsizing until you run
out of room. After this, it starts shrinking them to all fit on the
screen, then once it hits the minimum tab size it refuses to shrink
the tabs any more and adds a scroll widget to the tab bar.
I think I'd be fine with such a scheme, but I don't know whether it's
remotely feasible.

> My big complaint with the current
> scheme is I can't fit more than 4-5 tabs on my screen which makes tabs
> not-so-useful.  Right now I have 20 tabs in each of my Chrome and
> Pidgin windows, and I love it:)

To be honest, I don't think I've ever needed to have more than about 5
or 6 tabs open in Meld, so I obviously use it differently. :)

>>>  filediff: Make non-writable files uneditable
>>>  vcview: Make version control base files un-writable
>>
>> I agree that we need to do something better with non-writable files,
>> but I don't know that this is it. I think it's not that unusual to
>> want to make changes to an uneditable file in order to get diffs to
>> match up better. I often do something like this in three-way merges.
>>
>> There are things that could be done here though: we could make them
>> uneditable by default, and have a small unlock button; or we could
>> keep them editable, but have a InfoBar-like warning pop up to indicate
>> that the file can't be saved.
>
> The main motivation for the "un-writable" changes were to prevent
> users from modifying and saving a temporary file during a Vc diff.
> I've seen co-workers edit the left side of a VcView FileDiff and save
> it (which just saved a file in /tmp that would soon be deleted), then
> be confused about why the modifications they made in meld disappeared.
>  I'm fine with ignoring the "filediff: Make non-writable files
> uneditable" patch and just including "vcview: Make version control
> base files un-writable" one.  This would still allow you to tweak
> unwritable files during diffs, but wouldn't allow people to save
> temporary files during a Vc diff.  Thoughts?

That sounds sensible to me. I've pushed the non-writeable VC temp files patch.

cheers,
Kai