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

[Peter Tyser <ptyser gmail com>]

> I'm keen to see something happen with the tabs. I like where you're
> going with these patches, but in some cases I'm not convinced that
> it's the right way to fix the problem. I'm going to respond very
> quickly to these here, rather than in the individual patches.
>
>> Peter Tyser (7):
>>  Only show tabs if there are more than 1
>
> I'm of two minds about this. Getting rid of clutter is great. However,
> I personally think that tabs should always be shown in any application
> where you expect users to use multiple tabs. For me, this means that
> starting with a VcView or a DirDiff should show tabs; starting with a
> single FileDiff... probably shouldn't?
>
> Anyway, discussion required I think.

I like your suggestion of unconditionally showing tabs for VcView or
DirDiffs, and not showing them for a single FileDiff.  I'll send a
follow up patch.

>>  Allow reordering of tabs
>
> This is a gtk 2.10 feature, so can't go in to current head. I'm happy
> to put it in post-1.4 though.

I can make it conditional on gtk.pygtk_version() >= (2.,10,0) if you'd
prefer.  Either way is OK by me.

>>  Update handling of tab labels and tooltips
>
> I like the tab tooltip changes, but I'm not convinced that this is the
> right way to deal with tab labels. I'd like to figure out what the
> minimum detail is that we can use to differentiate comparisons.
>
> 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

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.

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?  How long is "too
long" for the label?  Does "too long" depend on font size and the
static tab width?  Can this shortening code be shared with the same
code to shorten DirDiff and FileDiff paths?
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?  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.
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.

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)
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.

> 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.

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.

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...).  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:)

>>  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?

Thanks,
Peter