I’m trying to add a new feature to Meld where in the DirDiff view, I can select multiple files and generate a patch (without opening each diff into a new tab).
I added a menu option and an event handler that creates FileDiff objects from the selected files and passes them down to the PatchDialog.
When I call the FileDiff.set_files, it creates a task to load the files. When the files are loaded it creates another task to run the comparison. The problem is that those 2 task are only ran after the patch dialog is created. That results in the patch dialogue being initially empty. If I select the “reverse order” checkbox, it forces an update and then the text appears in the dialogue.
If I understand it correctly, my format_as_patch event handler creates the fileDiff objects, which in turn creates tasks to load the files and run the comparison. Then it creates the patch dialogue and calls run on it. At that point the format_as_patch event handler is done and the window is idle, which executes the on_idle event handler, which then executes the 2 task (load files which schedules and runs the comparison).
If my understanding is correct, how do I force either the load/compare tasks to run first or the patch dialog to update after the comparison is done?
I tried creating a task for the patch dialog.run method hoping that way it will be executed after the load/compare, but that’s not the case.
Here is my event handler for context:
def action_format_as_patch(self, *args):
pane = self._get_focused_pane()
if pane is None:
return
selected = self._get_selected_paths(pane)
difffiles = []
for row in selected:
row_iter = self.model.get_iter(row)
row_paths = self.model.value_paths(row_iter)
gfiles = [Gio.File.new_for_path(p)
for p in row_paths if os.path.exists(p)]
doc = FileDiff(len(gfiles))
doc.set_files(gfiles, None)
difffiles.append(doc)
dialog = PatchDialog(difffiles)
self.scheduler.add_task(dialog.run())
I tried creating a task for the patch dialog.run method hoping that way it will be executed after the load/compare, but that’s not the case.
Yeah this won’t work because the scheduler is per-comparison, so when you create those FileDiffs they have their own schedulers that get run independently of the one you’re using, which is tied to the DirDiff.
I don’t really have an easy suggestion for you here, since none of this was built to do what you’re trying to do. If I were to try this, I would probably look at adding a diff-completed signal or something to FileDiff and then have a task in your DirDiff handler that waited for all of those to fire. However, I really don’t know whether this will work out… it’s just what I would try.
I came up with 2 different approaches, but I can’t decide which one is better:
Pass in a flag into set_files that forces the data loading / comparison to all be synchronous. Once everything is loaded, I just pass in the FileDiffs to the patch dialogue and I should be good to go.
Split the format_as_patch handler into 2 parts - 1 that loads the data and does the comparisons, and another that creates the patch dialogue. This gets complicated pretty quickly because I need to ensure that all selected files have been loaded - that means each will send a signal that it finished loading / comparing, then I have to accumulate those and ensure I only call the “create_patch_dialogue” handler, when they all report back.
I’m leaning towards #1. I would need to run “format_as_patch” in a separate thread so I don’t block up the GUI. Once the data is loaded and comparisons are ran, I can schedule a task to create the pop up patch dialog.
Assuming your intention is to get this merged into Meld, I would advise against option 1. The thing about the make-it-synchronous flag to set_files is that it doesn’t make any sense to do outside of this specific context, and I’m very hesitant to add behaviour like that for special corner cases.
I would suggest that there’s a third option which is probably better, which is to not create a FileDiff at all. I’ve just refreshed my memory of the patch dialog, and I don’t think it works the way you want it to.
For a start, there’s no multi-file support there, that would have to be added. More importantly though, it doesn’t actually use the result of the FileDiff comparison to create its patch. It just uses the FileDiff objects to extract the loaded text from their buffers.
I feel like it would probably be easier and cleaner to change PatchDialog to not require an actual FileDiff and instead just take a list of MeldBuffer objects. This would mean that you could in theory add a new class constructor that took e.g., a list of list of MeldBuffers and then did the file loading internally.
There may well be other issues I haven’t thought about with this approach, but it feels like it’s closer to what you’re actually trying to achieve.