On 30 March 2012 01:59, Piotr Piastucki <[email protected]> wrote: > Hi, > > I sent a small patch that added a simple patch view mode to meld a couple of > moths ago. Here is a somewhat improved version that makes it possible to > review patches and apply them to existing source code folder. Some info > about the usage can be found at http://piastucki.bdl.pl/meld/patch/. From > the technical point of view it works more or less as follows: > 1) patch file is parsed (patchparser.py) > 2) changes are applied in memory (patcher.py) > 3) new UI to review patches is provided (patchview.py, patchdiff.py) > > Any comment, questions and ideas for improvements are welcome.
This looks nice, and I think it may well be a good idea. I'd like to hear what other people think, whether they would find it useful, etc. A few issues I noticed when testing it: - The previous/next change shortcuts and buttons don't work. Like file comparison, we should support Alt+Up/Down and Ctrl+E/D and make it skip between chunks. - If I untoggle Flatten, the tree display breaks, maybe because I wasn't applying it to the current directory? The tree ended up looking like "0002-Enable-patch-mode.patch" -> ".." -> "meld-test" -> "" -> "meld" -> "meldwindow.py". I think some path canonicalisation is in order? - Similar thing in the Location column; there are double directory separators showing. - I don't really see the point of the 'Show matched chunks' button. Could you explain why I'd care to see that detail? - I can change the patch file name on top, but it doesn't do anything except change the tree top level name. I think we should just not support changing the patch file from there, and remove the entry; change it to a label or something. Better yet, if we've got a patch with a commit message, display that. - Scrolling seems to becomes permanently unlocked if the files are the same. - Our command line interface is already too overloaded, so I'd like to see this mode require an explicit --patch option, rather than guessing what to do when we see a .patch file. However, these are mostly minor issues. The main thing that would stop me from merging this patch at this stage is the ~550 lines of patch parsing and handling code, which makes me nervous. I'll have to think about this. While I hate shelling out to patch, looking at other solutions makes me think that this isn't really all *that* stupid. Maybe you've thought about this already, but I'd like to know whether shelling out is an option, and if it's not, how robust is the patch parsing? what kinds of patches does it handle, etc. Either way, it would be great to see this code and any successors attached to bugzilla somewhere. cheers, Kai _______________________________________________ meld-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/meld-list
