-
Notifications
You must be signed in to change notification settings - Fork 220
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
Compare snapshots: Don't use "false" as fallback diff command #1662
Comments
This is my idea of the fix, but I have a question (which probably does not make too much sense): |
Hello Kosta, Your approach seems correct to me. I would suggest a slightly refactoring like this:
And the dialog:
I would treat this as an edge case. The users are responsible for this them self. When they want a terminal based diff output they have to use the correct commands (some bash/sh, termemu magic). Just ignore this case.
We could check the output on stdout when using subprocess.run(). But this is to much code and complexity for an edge case (1 user in 100.000 maybe). My intention is to simplify the the old BIT code where ever it is possible to improve its maintainablity. 😄 |
Changed initial `DIFF_CMD` and `DIFF_PARAMS` creation, changed `diffCmd` checking mechanism and added better error description
… dialog - Select default diff command (meld, kompare) if none is setup but only if present on the system - Compare snapshots dialog reflect diff command settings via disabled/enabled compare button. - Improved and more clear messages to the user about problems with diff command setup. Fix #1662 Contributed by @stcksmsh (Kosta Vukicevic) --------- Co-authored-by: Christian Buhtz <c.buhtz@posteo.jp>
backintime/qt/snapshotsdialog.py
Lines 34 to 42 in ed282ad
Based on report on bit-dev mailing list.
Problem
An extern application is used to compare snapshots. If
meld
andkompare
as such tools are not installed BIT is falling back tofalse
. Be aware thatfalse
is notFalse
and thatfalse
is a valid command in a *nix shell (see$ manpage false
). This makes no sense from users perspective. The "Compare" button just does nothing in this case. No reaction. No response to the user.BIT doesn't check for
false
but just checks for availability of the configured diff command.backintime/qt/snapshotsdialog.py
Lines 336 to 340 in ed282ad
Solution
The diff command should be empty (or None) if meld or kompare are not available. And the Compare button should check for emptiness.
Also the error message could be improved. A simple "Command not found" is not user friendly and to technical.
The text was updated successfully, but these errors were encountered: