Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for calling different diff tools. #10

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OscarL
Copy link

@OscarL OscarL commented Dec 26, 2022

This is a first, rough attempt, at implementing support for different diff utilities. Current options are:

  • Internal
  • Pe "diff mode" (needs Pe's PR #85)
  • mcdiff
  • kdiff3 (untested)
  • vimdiff (untested)

Defaults to Internal for now, until saving/restore settings gets implemented., but remembers last selected choice.

Opening this as a Draft, as to gather some feedback, to see if this sounds more or less OK, or I'm missing the mark by too much.

I guess we could allow adding custom diff tools by reading "tool_name=cmd_line %file1 %file2" or similar from a settings file. For now, mostly due to my limited skills (and short attention spans)... that will be left out as an "exercise for the reader" :-)

Preview (from before mcdiff was added):
PonpokoDiff

@OscarL OscarL force-pushed the external_diff_tool branch 2 times, most recently from 811663e to 005181a Compare December 26, 2022 03:32
This is a first, rough attempt. Options are:
 - Internal
 - Pe "diff mode" (pending Pe's PR #85)
 - kdiff3 (untested)
 - vimdiff (untested)

Defaults to Internal for now, until saving/restore settings gets
implemented.
@humdingerb
Copy link
Member

At the moment, we can only be sure that "Internal" and "Pe diff" is available (Pe being bundled with Haiku, may change in the future, so better check on that, too).

I think the app should:
a) check which of the diff tools are installed and add only those to the menu.
b) show all diff tools in the menu and pop up an alert if a non-installed diff tool is selected. The alert could offer to install the missing tool via pkgman.

@OscarL
Copy link
Author

OscarL commented May 15, 2023

Thanks for your feedback @humdingerb! I agree with your suggestions, of course!

The main behaviour I wanted to achieve is, of course:

You select too files, "Open with... PonpokoDiff", it uses the diff tool you have selected without popping up other dialog.
But... with the code on this PR, that only works if the "select files" dialog is open first (by calling "Open with... PonpokoDiff" first on a single file, selecting a second, hitting "Diff them" button).

And while tonight I managed to hack it enough to finally do that "silently use the previously selected diff tool when selecting two files", I created plenty of buggy behavior too :-(

To make it work OK I think I would need to turn PonpokoDiff around a bit too much.

I only really care about its "OpenDialogForDiffs", and not so much about its diffing capabilities. PonpokoDiff is more centered around its "DiffWindows" than on this "OpenDialog".

Considering my limited skills. I'm starting to think that creating a new, simpler app, focused on just supporting external tools (Pe with PR#85 applied, mostly), might be my best option in the end (and simple enough as to let me learn/practice some BLayout GUI coding :-D).

Guess time will tell if I manage to do either... or if a better programmer implements cleanly the "proof of concept" on this PR :-D.

Thanks again!

@humdingerb
Copy link
Member

I once very briefly looked at changing this app to use layout management, but gave up for one reason or another.
If you look into creating a simple diff tool, it would be nice to be also able to compare a file against itself on another git branch. If that is possible...

@OscarL
Copy link
Author

OscarL commented May 16, 2023

it would be nice to be also able to compare a file against itself on another git branch

Agree! Will keep that good idea in mind, thanks.

I think writing a proof-of-concept for that, using some bash to script git, and the TrackRunner addon to do the file->Tracker->script association wouldn't be that hard.

@humdingerb
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants