On 09/09/2010 02:08 PM, Sascha Silbe wrote: > Excerpts from James Cameron's message of Thu Sep 09 03:24:36 +0200 2010: > >> Sorry, I used tabify in emacs, so as to force the diff to show the >> entire function as changed. I had said I "replaced tabs with spaces", >> but I should have said "replaced spaces with tabs". This is only >> because I don't know how to drive git diff well enough. > > This brings up something I have been brooding about for some time now: > > The default output of "git diff" often isn't very useful for > understanding a patch or even just merging changes. It's a low-level > representation and, when comparing Python code, often not even a good > one. > > I know git can be configured to use external "diff drivers" and "merge > drivers" (see also [6]), so maybe we can improve this. That's why I > started a new thread: Others might be aware of tools that can be used, > either as-is or after some integration work (or at least on their own). > > > There are two different things that bug me about "git diff" when applied > to the Sugar source code: > > > 1. The built-in algorithm is generic (good) and always language- > agnostic (bad). For Python code, this often results in: > > I. Choosing unsuitable borders. > Example: > > @@ -81,15 +82,19 @@ def _async_copy(self, file_path, destination_path, > completion_cb, > unlink_src) > async_copy.start() > > - def retrieve(self, uid, user_id, extension): > + def has_data(self, object_id): > + file_path = layoutmanager.get_instance().get_data_path(object_id) > + return os.path.exists(file_path) > + > + def retrieve(self, object_id, user_id, extension): > """Place the file associated to a given entry into a directory > where the user can read it. The caller is reponsible for > deleting this file. > > """ > > This mixes up the changes to retrieve (only signature changed) and > adding a new method has_data(). When handling conflicts, the above > output is unnecessarily hard to understand and resolve. > What I would have liked to see instead is: > > + def has_data(self, object_id): > + file_path = layoutmanager.get_instance().get_data_path(object_id) > + return os.path.exists(file_path) > + > - def retrieve(self, uid, user_id, extension): > + def retrieve(self, object_id, user_id, extension): > """Place the file associated to a given entry into a directory > where the user can read it. The caller is reponsible for > deleting this file. > > """
FWIW, I understand your example and the frustration well. Have been put off by it myself. I often use the diff and the copy of the original code and/or final code near by to understand bigger refactorings. Which is not very handy of course. So far, I have not come across better tools but will let you know if I see better. Regards, Simon _______________________________________________ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel