Internal OpamCmdliner lagging behind, missing fixes in upstream cmdliner

[jberdine]

What is the expected future of the internal OpamCmdliner copy of cmdliner? I ask because it is lagging behind upstream cmdliner and missing fixes. For example I am missing the fix for dbuenzli/cmdliner#191 that is in cmdliner but not opam.

[dbuenzli]

While there are a few other changes that the devs may care about, notably revamped error messages with ANSI styling, the main point of contention has been the drop of prefix specification.

Unless there is a bug in cmdliner, simply upgrading the sources to cmdliner 2.0 followed by this patch:

diff --git a/src/cmdliner_trie.ml b/src/cmdliner_trie.ml
index 4b520ff..965dfab 100644
--- a/src/cmdliner_trie.ml
+++ b/src/cmdliner_trie.ml
@@ -83,9 +83,4 @@ let of_list l =
   let add t (s, v) = match add t s v with `New t -> t | `Replaced (_, t) -> t in
   List.fold_left add empty l
 
-let legacy_prefixes ~env = match env "CMDLINER_LEGACY_PREFIXES" with
-| None -> false
-| Some s ->
-    match String.lowercase_ascii s with
-    | "true" | "yes" | "y" | "1" -> true
-    | _ -> false
+let legacy_prefixes ~env = true

would allow opam to benefit of all the cmdliner 2.0.0 improvements.

[kit-ty-kate]

We don't have any formal plans at the moment, but this subsection of the code isn't that different to any other piece of opam code in that it will be fixed if there is a bug. We'd like to avoid diverging from upstream 1.3.0 too much for now but we don't expect that many bugs (cmdliner has a good track record of being quite reliable) and dbuenzli/cmdliner@ce6e3a1 seems fairly self contained so i'm personally happy to apply it (PRs welcome if you have some spare time)

As for the future, i'm hopeful to be able vendor the latest version with minimal changes and to keep it up-to-date, but we haven't really talked about it yet so we'll see when we have some time to look at that.

[WardBrian]

As for the future, i'm hopeful to be able vendor the latest version with minimal changes and to keep it up-to-date, but we haven't really talked about it yet so we'll see when we have some time to look at that.

I would be happy to take on the brunt of the work to make this possible (I recognize that reviewing is also work, so my volunteering alone may still not be enough!)

[rjbou]

Discussed in dev meeting: we'll definitively backport the bug fixes (2.6 milestone). For later, we should be able to vendor 2.1 cmdliner with @dbuenzli proposed patch. It'll require some work on opam-client dependencies to upgrade.

[dbuenzli]

Note if you want to use the completion stuff with the patch you should rather vendor the future 2.1.1 (see @WardBrian's linked issue).

[WardBrian]

The curious can peruse WardBrian/opam@update-cmdliner-vendor...WardBrian:opam:cmdliner-completion-demo to see my scratch pad for completions built on a vendored copy of 2.1.0 plus the above legacy_prefixes patch and the fix for dbuenzli/cmdliner#262

There is certainly some room for improvement in my code, but it's already handling quite nuanced cases, such as opam remove <TAB> and opam remove --switch some_other_switch <TAB> suggesting different sets of packages. I think I have completions for switches, repos, and packages working in all the expected places, including some not covered by the current complete.sh.

When the time is right I can break this up into smaller patches and submit them. Beyond what is already in that branch, I don't think there is a ton of extra work besides some small dune config (to replace the current complete.sh with the result of running the cmdliner tool) to get really high-fidelity completions.

Comparison

$ # current completion script
$ opam repostitory <TAB>
           add        --help     list       Note:      priority   remove     set-repos  set-url 
$ # note that the "Note:" above does really get suggested!

$ # new completion script
$ ./opam repository <TAB>
                       --confirm-level        --help                 --no-aspcud            --rank                 set-repos              --use-internal-solver  --yes
-a                     --criteria             --ignore-pin-depends   --no-auto-upgrade      --readonly             set-url                -v                     
add                    --cudf                 --json                 --no-self-upgrade      remove                 --short                --verbose              
--all-switches         --debug                -k                     --on-switches          --root                 --solver               --version              
--best-effort          --debug-level          --kind                 priority               -s                     --strict               -w                     
--cli                  --dont-select          list                   -q                     --safe                 --switch               --working-dir          
--color                --git-version          --no                   --quiet                --set-default          --this-switch          -y