-
-
Notifications
You must be signed in to change notification settings - Fork 389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix noninteractive mode still requires user interaction #5754
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't cover every case, for example with auto_remote
when a ref is found in multiple remotes.
I think it would be more effective to add a noninteractive
parameter to flatpak_resolve_*()
in app/flatpak-builtins-utils.c
.
app/flatpak-builtins-install.c
Outdated
@@ -568,6 +568,8 @@ flatpak_builtin_install (int argc, char **argv, GCancellable *cancellable, GErro | |||
g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, _("Nothing matches %s in remote %s"), id, remote); | |||
return FALSE; | |||
} | |||
else if (refs->len >= 2 && opt_noninteractive) | |||
return flatpak_fail (error, _("Multiple refs match '%s', unable to proceed in non-interactive mode"), argv[1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return flatpak_fail (error, _("Multiple refs match '%s', unable to proceed in non-interactive mode"), argv[1]); | |
return flatpak_fail (error, _("Multiple refs match ‘%s’, unable to proceed in non-interactive mode"), id); |
argv[1]
isn't necessarily the right string.
Flatpak isn't very consistent about quoting in messages, but fancy single quotes seem to be the most common.
…s or installations match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I didn't expect most of the flatpak_resolve_*()
functions to be able to immediately short circuit, but I still think it makes sense to centralize it in the functions that show the prompts.
I could also bikeshed about the opt_
prefix on the parameter name, but I don't want you to waste time making trivial changes until a maintainer has a chance to look at it. (I have no idea why opt_search_ref
is named like that, it's not even an option...)
I agree. But I stick to the present style because that's not what my pr is about.
I also thought about requiring to specify repo when |
Related to issue #3848
When non-interactive mode is enabled, install and uninstall commands still prompted you if multiple refs match. In my PR, these commands fail if the argument turns out to be ambiguous.
To prevent this error, scripts should specify the repo and the exact name of needed app/runtime:
flatpak install --noninteractive flathub app/org.videolan.VLC/x86_64/stable
. In case there is any ambiguity, an error like this is printed:When multiple refs with the substring "doom" are installed: