Parameter -c is no longer repeatable

[pst-propharma]

We are working with a cvs Repository and have used diffuse for many years to view changes in commits. Since the release of Version 0.8.2 our tools no longer function as expected and we are forced to downgrade to the older version 0.7.7.

From our tools we are running: diffuse -c 1.16 <file1_path> -c 2.3 <file2_path> -c 24.3 <file3_path>

In Version 0.7.7 this opens cvs File Version difference from 1.15 to 1.16 for file 1, from 2.2 to 2.3 for file 2 and so on. In Version 0.8.2 this opens cvs File Version difference 24.2 to 24.3 for all the files, even if these Versions do not exist. And since it cannot find this version, it fails to show the diff. This is on Fedora 38 and 37.

If this is the intended behavior after the upgrade, we would be very interested to know, how we can achieve a similar result.

Thank you for the help.

[MightyCreak]

I'll take a look. Thank you for reporting this bug.

[Ansa211]

I have the same problem with repeated -r.

[pst-propharma]

I'll take a look. Thank you for reporting this bug.

Thank you

[glasswings]

Bisected using Git and -r

./inst/bin/diffuse -r $(git rev-parse v0.7.6) CHANGELOG.md -r $(
git rev-parse v0.7.7) CHANGELOG.md

Good: both tabs have commit hashes visible Bad: right tab is just CHANGELOG.md (from working directory) regression due to [v0.7.7-8-g8e32f88] Use GTK3's GApplication/GtkApplication #178 (#181)

[glasswings]

I see two approaches a fix could take:

• restore the original argument parser
• use Gtk's parser to handle positional arguments in a way that's compatible with v0.7.7

I'm not yet sure that Gtk can handle positional arguments. -r <rev> and -c <rev> were syntactically bound to the next filename, which is not something that GUI applications typically do with their command line arguments. So I lean towards restoring the handwritten parser, but I'll try to find good documentation on command-line parsing with Gtk's tools.

[MightyCreak]

So I finally have got time to understand more the issue. First, indeed, v0.8.0 introduced a regression with the CLI args.

The issue

So the -c (or --commit) option was actually working in a peculiar way. That wasn't obvious even when reading the old --help:

  ( -c | --commit ) <rev>          File revisions <rev-1> and <rev>

The way it worked, it seems that some options (such as -c) were applied to the next file given in the arguments list, but some options weren't.

This is how I use -c and it still work with v0.8.0+:

• This worked: diffuse -c 4512fa1 CHANGELOG.md
• But this worked as well: diffuse -c 4512fa1 (giving all the files changed in this commit)

But I wasn't able to repeat -c with a git repo, but apparently @pst-propharma could with cvs. I didn't know we could repeat this option as the source code handling the options is so bad. That's why I'd prefer not to keep the previous CLI behavior as it is very much not standard at all.

But now: how can we fix this? And/or is it fixable using the Gtk.Application arguments?

The suggested solution

The Gtk.Application arguments brings standardization to the CLI. But for sure, it implies to change how we use the options now.

So far, I am looking at then documentation here: https://amolenaar.pages.gitlab.gnome.org/pygobject-docs/Gio-2.0/class-Application.html#gi.repository.Gio.Application.add_main_option

We've seen that the -c works very well, as long as we don't repeat the option. So I'm thinking maybe we could use GLib.OptionArg.STRING_ARRAY and enable the use of multiple -c in the command-line.

The issue would be that -c expected 2 arguments before (which is not standard at all). So how about joining the 2 arguments with a character, like so -c <rev>(,<file_path>).

So we would have these use cases:

# show the changes at rev 1.16
diffuse -c 1.16

# show the changes in file1 at rev 1.16
diffuse -c 1.16,FILE1

# works also with this syntax, but only with one file
diffuse -c 1.16 FILE1

# show the changes in file1 at rev 1.16, file2 at rev 2.3, and file3 at rev 24.3
diffuse -c 1.16,FILE1 -c 2.3,FILE2 -c 24.3,FILE3

# optional, we could add multiple files, maybe?
# show the changes in file1 and file2 at rev 1.16, and file3 at rev 24.3
diffuse -c 1.16,FILE1,FILE2 -c 24.3,FILE3

The issue would be for filenames with a , in it, but we might find a way to escape these characters somehow (like preceding them with \).

Would it be acceptable?

[MightyCreak]

The issue with this solution, is that apparently, before, you could stack options until the code reached a file path, like that:

# show the changes in FILE1 at rev 1.16 using UTF-8 encoding,
# and FILE2 at rev 2.3 using encoding US ASCII
diffuse -e utf8 -c 1.16 FILE1 -e us_ascii -c 2.3 FILE2

I wonder now if there were more options that could be stacked like that :fearful:

[Ansa211]

Just a quick note, I think you (@MightyCreak) are probably aware of it: other repeated options (namely -r and -t) stopped working due to the introduction of Gtk.Application argument parsing. I guess any introduction of a new behaviour will pertain to those options as well.

• -t must be repeatable (that's the whole point of it!) and it should stack with all other options up to the next -t (#213)

• -r should be repeatable and probably should distribute the nearest filename over multiple appearances of -r:

  • (in 0.7.7) diffuse -r rev1 file -r rev2 file someOtherFile opens three panes, one for file @rev1, one for file @rev2 and one for someOtherFile @ working copy it is really annoying that if one wants to compare two revisions of the same file, one has to repeat the filename
  • (???) diffuse -r rev1 -r rev2 file someOtherFile ideal/shorter form of the same

Also related are issue #190 and pull request #212 . In particular, diffuse -c rev1..rev2 for opening a full diff between two revisions (one tab per file changed) is a really nice and useful shorthand.

[pst-propharma]

Thanks for looking into it, @MightyCreak.

I can say that your proposal here certainly would solve all our issues:

# show the changes in file1 at rev 1.16, file2 at rev 2.3, and file3 at rev 24.3
diffuse -c 1.16,FILE1 -c 2.3,FILE2 -c 24.3,FILE3

CVS just doesn't have a hash over multiple files, that means we kind of have to rely on this repeating -c solution. Currently we use Version 0.7.7 and in some cases Meld.

Syntactically we do not have a strong opinion on how it should work. All suggestions we come up with, are of similar nature, maybe replacing the comma with a different character.

[MightyCreak]

Since I won't be able to fix this issue in a timely manner and that some people are blocked because of it, I am OK with putting back the old arguments system. If someone wants to take it.

It would mean to revert the new argument management (add_main_option) here: https://github.com/MightyCreak/diffuse/commit/8e32f883ec5987d4a10b8a1186b472fa8457f698#diff-6b03446edc949963f04a7c3e5065953e173766ff54ac57f3ebd58095d2ca4bb0R1825

And ensure it works as before.

Does someone wants to take a stab at it?

I'll create an issue to try and have something a bit more standard, while keeping the same functionalities as before.