GNOME Bugzilla – Bug 665154
Offer to open binary files with appropriate program in file view
Last modified: 2015-09-25 21:42:14 UTC
It would be great to be able to open non-text files (e.g. from within a Directory Comparison) with the appropriate program, ie the program that's set as preferred for the file's mimetype, e.g. an .xcf file with Gimp, .svg with Inkscape etc. Currently, meld tries to open any type of file with gedit which naturally fails non non-text files.
You can actually do this already. Right-click a file and activate the "Open externally" option in the pop-up. (It might be called something different if you're not running current git HEAD.) Admittedly this isn't ideal. If we decide that we're seeing a binary file in the comparison, maybe Meld could get a different error page with a prompt to open each individual file in the default handler.
Not ideal? Itś a drag! Especially, since every file from a directory comparison refuses to open, no matter if it is text or not!
I'm sorry but I have no idea what you're trying to say. Does "Open externally" not open files? can you provide some details? OS, Meld version, etc.? Just double-clicking any file in a directory comparison should open a new tab and try to open it. If this doesn't work, then that's a bug. "Open externally" is to open files (mostly, files that Meld can't handle) in another program. If it doesn't do anything, then that's a bug too.
This should be relatively easy to implement, but there are two places where it makes sense. We could add an error handling path to FileDiff that prompts to open a file in an external program if it doesn't look like it's text. My preferred option at the moment would be to make DirDiff check to see whether it looks like we're going to be able to open the file in the file comparison sanely (that's the hard bit) and either prompt to open externally, or just do it. If we went with the just-do-it option, I think there would have to be some visual indication that we were going to do this... a cursor change or a mark on the file or... something. This might involve a bit of experimentation to get something that feels right.
Created attachment 310766 [details] [review] Open the file, if it is binary. I am new here and would like to learn and contribute more. So please let me know if I am doing anything wrong and help me correct it.
Thanks for the patch! That's definitely the right idea. One thing to think about here is that Meld runs on Windows and OSX as well as linux, so `xdg-open` won't always work. However, if you look at meld.melddoc.MeldDoc (which FileDiff is a subclass of) there's an `_open_files` method which you can use instead of the os.system call there. The other thing is that I think it's important that we allow the user to decide whether to open the file, since they may not expect us to just start another program. The best way I can think of to do this is to add an "Open externally" button to the notification that we show. You'll need a new method (and a new callback) to have a notification like this. You can look at FileDiff.notify_file_changed for an example of how to do this. As a minor point, when you submit patches it's easier to review if you leave out changes in the meld.pot file. Anyway, please feel free to ask more questions if I can help!
Created attachment 311080 [details] [review] open-binary-file-notification Done. Let me know if you need any changes. What should I do next?
Thanks, that looks pretty good. However, there's still a bug with files that aren't in the left-most pane. Because you're using a closure over the loop variable, that's not getting bound the way you'd expect in Python. I think you'll probably need something along the lines of: + def make_binary_callback(pane, filename): + def on_binary_file_open(msgarea, response_id, *args): + self.msgarea_mgr[pane].clear() + if response_id == Gtk.ResponseType.ACCEPT: + self._open_files([filename]) + return on_binary_file_open + + msgarea.connect("response", make_binary_callback(t.pane, t.filename)) or use functools.partial. Also, that patch was corrupted. I hand-applied it to check that it worked, but it's better if you can submit patches that can be directly applied, and mention which branch the patch is made against. You can read about the suggested setup for contributing at https://wiki.gnome.org/Newcomers/CodeContributionWorkflow. If that's too complicated for now, then just submitting a normally formatted patch is fine; it just makes it a bit harder to commit. Thanks again for the patch. It's almost there!
Created attachment 311664 [details] [review] Open_binary_files Hope this time I created the patch properly, I created it on branch open_binary.
Thanks! that looks good. I've pushed that patch to the 3.14 and master branches. https://git.gnome.org/browse/meld/commit/?id=32ab99ebf8afc37ff3bead2c9380a6a3897a9c14 As a very minor thing, it's somewhat better if you actually create the patch from a commit using `git format-patch` (see https://wiki.gnome.org/Newcomers/CodeContributionWorkflow#See_your_patch). If you do this, then whoever is looking at your patch can apply it with your authorship and commit message easily. I just went ahead and wrote a commit message anyway, just cause I didn't want you to have to wait on my limited spare time again. :) Anyway, thanks again!