Bug 73197

Summary: Extend rerun/refresh diff functionality to patch-viewing modes
Product: [Applications] kompare Reporter: Ingo Klöcker <kloecker>
Component: generalAssignee: Jeff Snyder <jeff-kdecvs>
Status: RESOLVED FIXED    
Severity: wishlist CC: bruggie, erbsenzahl, jeff-kdecvs, joshuakeel, kde-bugzilla, kjetil, opensource
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: patch from joshua keel
Updated patch. Hopefully ready to commit.

Description Ingo Klöcker 2004-01-22 01:10:33 UTC
Version:           3.2.90 (using KDE 3.2.0 RC1, compiled sources)
Compiler:          gcc version 3.3 20030226 (prerelease) (SuSE Linux)
OS:          Linux (i686) release 2.4.20-4GB

After changing the configuration (e.g. the exclude pattern) I'd like to rerun diff. This also makes sense if some compared files have been changed while Kompare is running.

Of course, rerunning diff is only possible if Kompare compared two files/directories, but not if Kompare uses stdin or a diff file (although the diff file could be reloaded; it could have been regenerated by the user).
Comment 1 Otto Bruggeman 2004-01-25 19:26:35 UTC
Most of the support for this is already there (see komparemodellist.cpp, in the slotDirectoryChanged() and slotFileChanged() methods) but it is not properly implemented since it is a pain to integrate properly. I'll see what i can do.
Comment 2 Jeff Snyder 2004-11-21 22:42:11 UTC
*** Bug 93516 has been marked as a duplicate of this bug. ***
Comment 3 Jeff Snyder 2004-11-22 04:45:57 UTC
making this a JJ, since it should be relativlely easy.

best to do this in kompare3_branch, where komparemodellist has been split into modelcontroller and modelcreator. I think the slots bruggie mentioned above were for some directory/file-monitoring code to do this refresh automatically, but I don't think we really need that.

This will mostly consist of:
* adding an action to kompare_part
* adding code to work out wether we can regenerate the diff
* adding code to call the right method on modelcreator to recreate the diff.

feel free to contact me on irc (#kompare,freenode) about this, or via email.
Comment 4 Otto Bruggeman 2004-11-22 22:33:27 UTC
Yes those methods were for automatic re-diffing... I think I removed most of them now (at least in my local check out) so dont use them. Should be fairly easy with Jeff's hints to implement this so have a go at it, there are at least a few more developers that want to have this option implemented.
Comment 5 Jeff Snyder 2005-01-03 22:18:34 UTC
adding 'komapare' to the summary because this one shows up in the JJ list.
Comment 6 Jeff Snyder 2005-05-24 00:30:32 UTC
Joshua Keel is taking a look at this bug, so I'm assigning it to him, adding myself as a CC, and attaching the patch Joshua just mailed to me..
Comment 7 Jeff Snyder 2005-05-24 00:31:45 UTC
Created attachment 11170 [details]
patch from joshua keel
Comment 8 Jeff Snyder 2005-05-24 01:14:59 UTC
Joshua: this patch is looking pretty good..
there's two things I'd like to see fixed before it's committed:
1. there needs to be a way of executing it - a menu entry in komparepart/komparepartui.rc would be good. probably in the File menu, for want of a better place.
2. the whitespace on the KAction still isn't right.. the second and third lines should have exactly the same number of tabs at the start as the first line, because they are supposed to align with it - this makes the alignment work at any tabstop setting.

When they're fixed, attach the new patch to this bug, and I'll give it the OK..
When the time comes to commit it, use CCBUG: instead of BUG: to avoid closing this bug.

This patch doesn't actually warrant closing this bug because it only covers the ComparingFiles and ComparingDirectories modes. It doesn't deal with the ShowingDiff, BlendingFile and BlendingDirectory modes, which would require the reloading of the patch URL. 

Extending the refresh to those situations will be fiddly because we can only do this when "m_modelCreator->openDiff"  was used instead of "m_modelCreator->openBuffer". We currently can't tell which was used, and fixing that would require adding another hack to the Kompare::Info system.

The Kompare::Info system really needs some reworking, so I'd rather put off extending the refresh capabilities until that's done. This is nontrivial, so this bug should have it's "JJ" tag removed once the first patch is committed.

 - Jeff
Comment 9 Otto Bruggeman 2005-05-25 02:13:24 UTC
The Kompare::Info system should be dropped asap and we should find a much better way of transferring data between both parts of the ui. I have already been thinking about creating one part and adding the nav widget part in it and showing it if comparedfiles > 1. Unfortunately I dont have time to do so myself. So if anyone wants to volunteer to do something about it I will see if I can help him or her as much as possible by answering any question he or she may ask.
Comment 10 Joshua Keel 2005-05-25 20:15:14 UTC
Created attachment 11200 [details]
Updated patch. Hopefully ready to commit.

This should fix the issues you were concerned about, Jeff. Specifically, the
KAction whitespace is fixed, and a method of executing the refresh has been
added. I added an icon to the end of the difference actions toolbar and an
entry in the 'Difference' menu. The standard F5 keyboard shortcut also works.
:)

BTW, nice to hear from you Bruggie. :)
Comment 11 Otto Bruggeman 2005-05-25 20:59:21 UTC
Patch looks good to me. Jeff, any further remarks ? Thanks for the work Joshua! Oh you can also add your name to the contributors in the KAboutDialog if you want to. Just create an entry there when you commit this patch if Jeff has no more wishes.
Comment 12 Jeff Snyder 2005-05-25 21:26:08 UTC
No more comments from me.. go ahead and commit (:
Comment 13 Joshua Keel 2005-05-25 21:59:32 UTC
SVN commit 418173 by jkeel:

Added the ability to refresh the difference view when files have been updated or changed.

CCBUG: 73197
GUI: Adds a 'Refresh Diff' menu entry, and an icon for the action in the 
toolbar. 


 M  +17 -0     kompare_part.cpp  
 M  +2 -0      kompare_part.h  
 M  +2 -0      komparepartui.rc  


--- branches/kompare3/kompare3_branch/kdesdk/kompare/komparepart/kompare_part.cpp #418172:418173
@@ -33,6 +33,7 @@
 #include <kinstance.h>
 #include <ktempfile.h>
 #include <kparts/genericfactory.h>
+#include <kstdaccel.h>
 //#include <ktempdir.h>
 
 #include <kio/netaccess.h>
@@ -179,6 +180,9 @@
 	m_diffStats       = new KAction( i18n( "Show Statistics" ), 0,
 	                                 this, SLOT(slotShowDiffstats()),
 	                                 actionCollection(), "file_diffstats" );
+	m_diffRefresh     = new KAction( i18n( "Refresh Diff" ), "reload",
+	                                 KStdAccel::reload(), this, SLOT(slotRefreshDiff()),
+	                                 actionCollection(), "difference_refreshdiff" );
 
 	KStdAction::preferences(this, SLOT(optionsPreferences()), actionCollection());
 }
@@ -190,11 +194,13 @@
 		m_saveDiff->setEnabled ( m_modelController->mode() == Kompare::ComparingFiles || m_modelController->mode() == Kompare::ComparingDirs );
 		m_swap->setEnabled     ( m_modelController->mode() == Kompare::ComparingFiles || m_modelController->mode() == Kompare::ComparingDirs );
 		m_diffStats->setEnabled( m_modelController->modelCount() > 0 );
+		m_diffRefresh->setEnabled( m_modelController->mode() == Kompare::ComparingFiles || m_modelController->mode() == Kompare::ComparingDirs );
 	} else {
 		m_saveAll->setEnabled ( false );
 		m_saveDiff->setEnabled ( false );
 		m_swap->setEnabled ( false );
 		m_diffStats->setEnabled ( false );
+		m_diffRefresh->setEnabled ( false );
 	}
 }
 
@@ -728,6 +734,17 @@
 	}
 }
 
+void KomparePart::slotRefreshDiff()
+{
+	if ( !m_info.localSource.isEmpty() && !m_info.localDestination.isEmpty() )
+	{
+		m_modelCreator->compareFiles( m_info.localSource, m_info.localDestination );
+		updateActions();
+		updateCaption();
+		updateStatus();
+	}
+}
+
 bool KomparePart::queryClose()
 {
 /*	if( !isModified() ) return true;
--- branches/kompare3/kompare3_branch/kdesdk/kompare/komparepart/kompare_part.h #418172:418173
@@ -152,6 +152,7 @@
 	void slotShowError( QString error );
 	void slotSwap();
 	void slotShowDiffstats();
+	void slotRefreshDiff();
 	void optionsPreferences();
 	void updateActions();
 	void updateCaption();
@@ -175,6 +176,7 @@
 	KAction*                 m_saveDiff;
 	KAction*                 m_swap;
 	KAction*                 m_diffStats;
+	KAction*                 m_diffRefresh;
 	KTempFile*               m_tempDiff;
 	struct Kompare::Info     m_info;
 };
--- branches/kompare3/kompare3_branch/kdesdk/kompare/komparepart/komparepartui.rc #418172:418173
@@ -21,6 +21,7 @@
     <Separator/>
     <Action name="difference_previous"/>
     <Action name="difference_next"/>
+    <Action name="difference_refreshdiff"/>
   </Menu>
   <Menu name="settings"><text>&amp;Settings</text>
     <Action name="options_configure"/>
@@ -37,5 +38,6 @@
   <Action name="difference_unapply"/>
   <Action name="difference_apply"/>
   <Action name="difference_applyall"/>
+  <Action name="difference_refreshdiff"/>
 </ToolBar>
 </kpartgui>
Comment 14 Mathieu Jobin 2006-06-02 12:56:33 UTC
I was about to create a duplicate of this bug.
and seems like the functionality is actually written.
as said on comment #12 ... "GO AHEAD AND COMMIT"

please.
Comment 15 Dirk Mueller 2006-07-30 12:46:43 UTC
*** Bug 107851 has been marked as a duplicate of this bug. ***
Comment 16 H. Peter Anvin 2007-11-02 21:50:20 UTC
This does not appear to be in kompare at least as of Fedora 7 (KDE 3.5.7).  Pretty please?
Comment 17 Raúl 2007-11-15 18:12:07 UTC
I think there could be some /unsinchronization/ between some main development branch and the official releases branch. Look https://bugs.kde.org/show_bug.cgi?id=104660

In these both we can see commits but no results in the officially released versions.

Thanks.
Comment 18 Kevin Kofler 2008-10-14 03:19:40 UTC
SVN commit 871152 by kkofler:

Kompare: Finally add support for refreshing diffs to the trunk. (Though it was possible to get that effect with the "swap and swap back" hack which I got all too accustomed to. ;-) ) Partly based on revision 418173 by jkeel from 3_way_kompare, also incorporates my followup (revision 871142), the actual implementation of the refreshing reimplemented to work in trunk.

CCMAIL: kompare-devel@kde.org
BUG: 73197

 M  +31 -1     komparepart/kompare_part.cpp  
 M  +3 -1      komparepart/kompare_part.h  
 M  +1 -0      komparepart/komparepartui.rc  
 M  +10 -0     libdiff2/komparemodellist.cpp  
 M  +2 -1      libdiff2/komparemodellist.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=871152
Comment 19 Kevin Kofler 2008-10-14 17:18:36 UTC
SVN commit 871329 by kkofler:

Kompare: Use icon-naming-spec-compliant name for the "Refresh Diff" action.

CCMAIL: kompare-devel@kde.org
CCBUG: 73197

 M  +1 -1      kompare_part.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=871329
Comment 20 Kevin Kofler 2009-05-14 01:33:02 UTC
*** Bug 192618 has been marked as a duplicate of this bug. ***