Parameter -c is no longer repeatable about diffuse HOT 10 OPEN

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.

from diffuse.

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?

from diffuse.

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 😨

from diffuse.

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.

from diffuse.

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

from diffuse.

I have the same problem with repeated -r.

from diffuse.

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

Thank you

from diffuse.

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)

from diffuse.

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.

from diffuse.

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: 8e32f88#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.

from diffuse.