Print shell-appropriate eval command on opam init

[freevoid]

This change is similar in spirit to 46eab70 (an open pull request marked as obsolete) with a slightly different implementation.

I'm new to OCaml and been following the instructions to install opam, during opam init it prints a shell-dependent line that will be added, but incorrectly prints eval $(opam env) (I use fish so that should be eval (opam env)):

In normal operation, opam only alters files within ~/.opam.

  However, to best integrate with your system, some environment variables
  should be set. If you allow it to, this initialisation step will update
  your fish configuration by adding the following line to ~/.config/fish/config.fish:

    source /Users/name/.opam/opam-init/init.fish > /dev/null 2> /dev/null; or true

  Otherwise, every time you want to access your opam installation, you will
  need to run:

    eval $(opam env)

  You can always re-run this setup with 'opam init' later.

Let me know if the change is not idiomatic, since I just installed opam and ocaml and have no previous experience with either.

[dra27]

Thanks for picking up the pieces of #3274! It's a better idea than I did in that PR to split the eval into two functions, but I would split it differently - I would have one function which works out the opam env command required (so takes all the extra parameters in to account) and then another command which takes that and turns it into the correct eval command.

I think you then want opam_env_invocation. That wants to be let opam_env_invocation : ?root ?switch ?(set_opamswitch=false) () = (* if root is Some x, convert it to --root=x, as eval_string does, etc. *) and essentially works out suffix in your current patch but with the opam env part too. eval_string would use that function instead of working out the --root= and --switch, etc. itself.

You then want a new function shell_eval_invocation which takes parameters shell and cmd (and is basically your current eval_string_simple). I think this addresses @AltGr's #3274 (comment) nicely since it's then not necessary to add --shell to the other commands.

If you want, you could also use those functions for a version of b4b3fca (which tries to the use the correct "guessed" eval string in help text output too)

[freevoid]

Hey David, thanks for the detailed comment!

I've tried to express your suggestions in 8340e6e, hopefully I didn't misunderstand.

In terms of using the functions for opamCommands.ml, I think I could export the shell_eval_invocation function and do something in the spirit of OpamEnv.(shell_eval_invocation shell "opam config env" |> Manpage.escape). If it sounds about right, I will go ahead and make the changes.

I see how b4b3fca used Manpage and ManSwitch to call different flavors of eval_string, I think we can keep it a bit simpler and just use shell_eval_invocation directly, so that eval \\$(opam env --switch=SWITCH --set-switch\\) becomes

OpamEnv.(
  shell_eval_invocation shell
    (opam_env_invocation ~switch:"SWITCH" ~set_opamswitch:true ())
  |> Manpage.escape)

[dra27]

My apologies for not responding to your message a bit more quickly, but thank you for rebasing and updating this in the meantime. The implementation is exactly what I had in mind - the suggestions below are just possibly to use OpamStd.Option to reduce some of the matches on options.

This is basically ready to merge - thanks for taking on my stale PR and improving it!

[dra27]

Slightly more concise as:

Suggested change

	let root =
	match root with
	| None -> ""
	| Some r -> Printf.sprintf " --root=%s" r
	in
	let switch =
	match switch with
	| None -> ""
	| Some s -> Printf.sprintf " --switch=%s" s
	in
	let root = OpamStd.Option.map_default (Printf.sprintf " --root=%s") "" r in
	let switch = OpamStd.Option.map_default (Printf.sprintf " --switch=%s") "" s in

[freevoid]

Thanks, applied (r and s converted to "root" and "switch")

[dra27]

This can be refactored slightly more clearly as:

let f sw =
  let sw_cur = ... (* etc. code from the `Some sw` case *)
in
OpamStd.Option.replace f switch

[freevoid]

Thanks, refactored

[freevoid]

Thank you for the review!
I've squashed the commits a bit as their number started to grow unnecessarily

[dra27]

Lovely, thank you!

[dra27]

(The mingw64 failure is nothing to do this PR)