Fix behaviour of menu and depexts in non-interactive environments #5295
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AltGr
commented
Sep 22, 2022
| and check_again t sys_packages = | ||
| let needed, _notfound = OpamSysInteract.packages_status sys_packages in | ||
| let needed, _notfound = OpamSysInteract.packages_status ~env sys_packages in |
There was a problem hiding this comment.
This one was well hidden, but at least one test caught it. Maybe the parameter shouldn't have been optional 😬
There was a problem hiding this comment.
Totally agree! Optional parameters should be avoided as much as possible IMO.
AltGr
commented
Sep 22, 2022
| let installed = OpamSysPkg.Set.diff sys_packages needed in | ||
| let t, sys_packages = | ||
| map_sysmap (fun sysp -> OpamSysPkg.Set.diff sysp installed) t, needed | ||
| in | ||
| if OpamSysPkg.Set.is_empty sys_packages then t else | ||
| (OpamConsole.error "These packages are still missing: %s\n" | ||
| (syspkgs_to_string sys_packages); | ||
| entry_point t sys_packages) | ||
| if OpamStd.Sys.tty_in then entry_point t sys_packages | ||
| else give_up ()) |
There was a problem hiding this comment.
Always avoid looping if we don't read from a tty
|
As mentionned in private, this is more restrictive when running non-interactively but better reflects what I had in mind when adding the menu. I am open to discussion if it has some unwanted effects though, esp. since different workarounds have been proposed in the meantime. |
Since this is a different approach to the same purpose, reverts: * ocaml#5156: bf37147 * ocaml#5257: d12f951 796b0bd 3681507
|
The new mechanism is good for me! clearer and more local |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The rationale is now to assume the "no" case in the menus whenever there is no input to read from.