Created attachment 132889 [details] [review]
Add indicator dialog when files are identical (for 1.2.1)

Simple patch to throw up a dialog when the files diffed are identical.  My patch seems to be a little too much like a hack, anyone know of a better way to do this?

I've just posted a patch series on the ML which updates the up/down buttons sensitivity according to actual presence of diffs.

I prefer that to another new evil dialog.

I'm not against it, but that should have an option that default to "no dialog"

Created attachment 132920 [details] [review]
pass meldapp to melddocs

Created attachment 132921 [details] [review]
allow app actions sensitivity to be modified

Created attachment 132922 [details] [review]
update up/down buttons sensitivity

I guess I'm used to the "Files are Identical" dialog, since thats the behavior of the diff tool I am attempting to wean myself from ;).  In fact, the behavior of this other tool is to throw a dialog that says that they're identical then ask whether or not you still want to view the file.  This tool, however, is not a tabbed interface like Meld is, so that step may not be necessary for Meld.

Created attachment 132967 [details] [review]
add missing vcview constructor call

Would only greying the up & down buttons be sufficient for you or you really want a popup dialog ?

An optional dialog that asks if user want to skip the diff would be useful for big files for example, as an optimization...

I think its a matter of how one's eyes are trained.  I could retrain my eyes to look in certain places on the screen to tell whether or not something is a match or not.  Greying the buttons would work, however, if the idea is to grey the buttons if you're at the top/bottom of the diff list (or the file), you get a bit of a false positive if there is only 1 diff, or if the file is less than 1 screen long.  

A dialog box, although a bit rough, is definitely in your face, blatantly obvious that these files are the same, there is no thinking involved.

Perhaps some other indicator?  Like maybe the tab be a special color?

The patches I propose here grey the 2 button iff there's no diff at all, so that would be a good indicator.

But you're right that the other related bug:
http://bugzilla.gnome.org/show_bug.cgi?id=533752
asks for a greying at the top or bottom, which would cause the greying to not be a good indicator of no diff in file for a single difference.

But as meld goes automatically to the first diff, you'll have that diff displayed
right in the textviews, so this may still be sufficient...

I still thinks the dialog is another (sensible) way to tell there's no diff, and as it would allows to skip displaying (and highlighting) big files, that could be implemented too...

Another idea that just popped into my head is that we could use the status bar to convey this information....a section of the status bar could say something like "Diff X of Y", where I'd know immediately if it said "Diff 0 of 0" the files are the same.

Yes, a status bar message should work.

When there are no diffs maybe the message can be made to stand out somehow, different color or bold?

Created attachment 133719 [details] [review]
diff status bar (for 1.3.0)

Quick diff status bar addition.  Shows Diff: X of Y in the status bar.  Unfortunately, it seems colors are difficult due to how things are drawn, but its a interesting concept.

I'm not that well versed in the internals of the diffing process and how the sequences work, so my calculations are probably pretty kludgy...any suggestions for cleaning that up would be appreciated.

I had to modify the patch somewhat to make it work, but nice idea.

How about reusing the existing status bar? i.e. concatenating with 
the cursor position info. I'd find it easier to locate the message
if it was near the corner.

Also what should happen when the user scrolls the cursor? Should the 
"diff X of Y" only be displayed when the cursor is inside a diff chunk?
This would mean warping the cursor when pressing Ctrl+d & ctrl+e
which is probably what the user wants anyway.

(In reply to comment #14)
> I had to modify the patch somewhat to make it work, but nice idea.

Can you post the changes, or send them to me?  Was it the code or the patch file itself?

> 
> How about reusing the existing status bar? i.e. concatenating with 
> the cursor position info. I'd find it easier to locate the message
> if it was near the corner.

Yeah, concatenating with the cursor location status bar section is possible of course.  I can just modify the signals a bit.  There was mention of coloring things, but from the looks of it, you cannot color status bars.  It seems to want to draw its own coloring, probably thanks to GTK's theming stuff...so for now we'll do without.

> 
> Also what should happen when the user scrolls the cursor? Should the 
> "diff X of Y" only be displayed when the cursor is inside a diff chunk?
> This would mean warping the cursor when pressing Ctrl+d & ctrl+e
> which is probably what the user wants anyway.
> 
I was just throwing out a POC idea, I was only changing the information when the buttons are being pressed, but to round out the solution, all of the bases should be covered.  What if you press the hot keys?  What if you scroll manually up and down in a file?  Should you be directly on top of a diff for it to trigger the change, or simply within the vicinity?  What if there are multiple diff's viewable on the screen, should there be some indication of such thing?

Also, there is a minor side effect of this code...as you merge changes and things become more similar, the "of Y" part changes, obviously.  Personally I think that should be OK...

And again, I'm not sure the work I did to collect this information is all that great.  After thinking about it, I think there should probably be more methods added to the diffing classes to help determine how many diffs there are, and for a given line number(?) which diff we're sitting on...I'm just not that comfortable with my calculations.

Created attachment 133797 [details] [review]
Warp text cursor within jumped-to chunk

Is that one OK to commit ?

BTW I'm still working on the Up/Down button sensitivity thing,
but there are a lot of cases to cover.

Yes, the cursor warp looks good - it's the first part of keyboard merging too, along with highlighting the current diff chunk.

Created attachment 133839 [details] [review]
Add diff info to doc status bar

Another stab at the diff status bar change.

This time the diff information is part of the existing doc status bar, I just increased the size of the bar a bit.

I added a couple of functions to the status bar class to aid in showing the tooltip information.  I was seeing a problem where using a button to move to the next diff was popping back to the wrong status information.

This patch is on top of the recently merged "cursor warping" patch, so I used the fact that the cursor is moving to determine where in the document we're at and which diff chunk we're sitting in.  So now, when you move the cursor by hand or using the next/prev hotkeys it works.  

There is only 1 bug with it...when warping to a deleted section, the cursor doesn't quite go to the location where the deletion occurred.  Because of that, the diff location does not update unless you move down a couple of lines to where the lines were actually deleted.

Yes I'm contemplating that bug and scratch my head to find a fix...

The bug is a little bit more than just for deleted sections, it
is wrong in the first pane (and probably in the third if there's
one)

I'll scratch a bit more before a reverting, but if I cannot find
it...

Well, looking at the next diff code, for "delete" and "replace" chunks, l0 would be the right place to put the cursor, but for "insert" chunks, l0 refers to the opposite pane.  On the pane the cursor is moving within l0 goes to the wrong line.

I changed:
buf.place_cursor(buf.get_iter_at_line(l0))
To:
buf.place_cursor(buf.get_iter_at_line(c[1]))

And my problem is resolved.  I don't know how this affects the issues you mention.

Actually, playing with it more, neither of the lines work right.  If you're cursor is in pane 1, you want to use pane 1's line information c[3] & c[4].  If your cursor is in pane 2, you want to use pane 2's line information c[1] & c[2].  My code that figures out the diff chunk the cursor is within isn't quite right either, since its not taking into account which pane the cursor was moved within.

Created attachment 135153 [details] [review]
Diff info in status bar.

Another shot at this one.

A couple of tweaks from the last patch.  The call to locate_chunk seemed to be returning the wrong chunk given the line.  Also, it seems that the place cursor call was placing the cursor 1 line past where it ought to.  Having said that, according to the chunk information, the cursor is in the right place, but due to the fact that the lines drawn and the coloring starts from the "bottom edge" of the given line, the cursor sits above the colored section upon warping.

So it ends up looking funny for inserted sections and replacement sections, but looks ok for deleted sections.

Also, this patch says "Diff: 1 of 4" if you're anywhere above the first diff.  Conversely, it says "Diff: 5 of 4" if you're below the last diff.  Which obviously looks funny.  How should it look?  Should we only show the diff info if your strictly within the bounds of a chunk? and leave it blank if the cursor is not?

I think that both of the ideas here -- sensitivity setting on up/down arrows, and chunk numbers in status bar -- are awesome and should happen, but I'd like to see something more obvious to address the original request. Two files being identical really is a big-flashing-notice kind of situation. I agree that pop-up dialogs are a no-no, but I think that a message area (ala gedit, or hotssh for the python version) is a good choice for this kind of display.

I'm attaching a patch that adds a copy of hotssh's msgarea.py to our tree, incorporates the message area into the glade files, and uses the message area to show a dismissible message when files are identical.

In the future (pending the app-centric gtk.Action issues that Vincent has already run into) we could add several enhancements to this. Obvious choices include a reload button, and a different message if the files are only the same *after* filtering, with a "Re-diff without filters" button. The message area could also be used for things such as file-change notification, and probably other stuff that I haven't thought of yet.

Created attachment 137541 [details] [review]
Patch using a message area to notify user about identical files

The msgarea patch looks great. How about an accelerator to close the area?

This reminds me a little of the file/folder entry activation behavior conundrum. If the browse... button popped down a FileChooserWidget for every pane, all panes could reload when the popdown was closed.

> The msgarea patch looks great. How about an accelerator to close the area?

Added and committed. The appearance is a little odd - it's only on the first
area's button to avoid conflicting mnemonics. This looks slightly strange, but
works correctly.

> This reminds me a little of the file/folder entry activation behavior
> conundrum. If the browse... button popped down a FileChooserWidget for every
> pane, all panes could reload when the popdown was closed.

That's a really interesting idea. It sounds like the interaction should work
well and be more obvious than the current situation. FileChooserWidgets are
quite big, however, so having three (or even two) side-by-side would probably
have implications for Meld's required minimum window size.

> That's a really interesting idea. It sounds like the interaction should work
> well and be more obvious than the current situation. FileChooserWidgets are
> quite big, however, so having three (or even two) side-by-side would probably
> have implications for Meld's required minimum window size.

I'd hate to impose a (large) window minimum size. We may need a custom container widget which could morph between a hbox and a notebook as the window size changed. Bonus if it can morph without abruptly popping.

I've created a new bug to track cursor and scrolledwindow warping bug / enhancement:

http://bugzilla.gnome.org/show_bug.cgi?id=588720

obsoleted the patches in this bug since the new ones in 
http://bugzilla.gnome.org/show_bug.cgi?id=588720
are the latest versions

I obsoleted my patches, since they're pretty old and I like the new message area notification.

Great, then I think we're done here.