[pacman-dev] noconfirm trivial bug - prompt printed but unused/unnecessary
Hi all I'm still getting my head around git and pacman-dev's workflow - I know its not that complicated, just need a bit of time. When using --noconfirm the prompt is presented (odd part) and the user (rightly) is not given an opportunity to confirm:
[ brendan@swift : 12:36:40 : ~/src/pacman ] :) sudo pacman -Sc --noconfirm Cache directory: /var/cache/pacman/pkg/ Do you want to remove outdated packages from cache? [Y/n] <------ removing old packages from cache... done. Database directory: /var/lib/pacman/ Do you want to remove unused repositories? [Y/n] <------ Database directory cleaned up [ brendan@swift : 12:36:57 : ~/src/pacman ] :) Best fix I can think of is to just mention that we're using a default because the --noconfirm flag has been used. The alternative (which is less informative for the user) is to not present the prompt at all.
I've inserted 2 strings below. Assuming this is the right change to make, what else would I need to do to ensure that this is compatible with the i18n measures already in place? util.c, lines 641-680:
/* presents a prompt and gets a Y/N answer */ static int question(short preset, char *fmt, va_list args) { char response[32]; FILE *stream;
if(config->noconfirm) { stream = stdout; } else { /* Use stderr so questions are always displayed when redirecting output */ stream = stderr; }
vfprintf(stream, fmt, args);
if(preset) { fprintf(stream, " %s ", _("[Y/n]")); } else { fprintf(stream, " %s ", _("[y/N]")); }
if(config->noconfirm) { fprintf(stream, "\n"); Replace the 'fprintf(stream, "\n");' line above with something like: if(preset) { fprintf(stream, "%s", _("noconfirm option set, defaulting to Yes\n")); } else { fprintf(stream, "%s", _("noconfirm option set, defaulting to No\n")); } return(preset); }
if(fgets(response, 32, stdin)) { strtrim(response); if(strlen(response) == 0) { return(preset); }
if(!strcasecmp(response, _("Y")) || !strcasecmp(response, _("YES"))) { return(1); } else if (!strcasecmp(response, _("N")) || !strcasecmp(response, _("NO"))) { return(0); } } return(0); } --
__________ Brendan Hide
On Wed, May 20, 2009 at 2:07 PM, Brendan Hide
Hi all
I'm still getting my head around git and pacman-dev's workflow - I know its not that complicated, just need a bit of time.
When using --noconfirm the prompt is presented (odd part) and the user (rightly) is not given an opportunity to confirm:
[ brendan@swift : 12:36:40 : ~/src/pacman ] :) sudo pacman -Sc --noconfirm Cache directory: /var/cache/pacman/pkg/ Do you want to remove outdated packages from cache? [Y/n] <------ removing old packages from cache... done. Database directory: /var/lib/pacman/ Do you want to remove unused repositories? [Y/n] <------ Database directory cleaned up [ brendan@swift : 12:36:57 : ~/src/pacman ] :)
Best fix I can think of is to just mention that we're using a default because the --noconfirm flag has been used. The alternative (which is less informative for the user) is to not present the prompt at all.
If you add another sentence, it's redundant imo. pacman already shows the default answer in upper case, so we already know that Yes was chosen in both cases, automatically because of --noconfirm.
Brendan Hide wrote:
Hi all
I'm still getting my head around git and pacman-dev's workflow - I know its not that complicated, just need a bit of time.
When using --noconfirm the prompt is presented (odd part) and the user (rightly) is not given an opportunity to confirm:
[ brendan@swift : 12:36:40 : ~/src/pacman ] :) sudo pacman -Sc --noconfirm Cache directory: /var/cache/pacman/pkg/ Do you want to remove outdated packages from cache? [Y/n] <------ removing old packages from cache... done. Database directory: /var/lib/pacman/ Do you want to remove unused repositories? [Y/n] <------ Database directory cleaned up [ brendan@swift : 12:36:57 : ~/src/pacman ] :) Best fix I can think of is to just mention that we're using a default because the --noconfirm flag has been used. The alternative (which is less informative for the user) is to not present the prompt at all.
I'd be in favour of the less informative route. I.e. sudo pacman -Sc --noconfirm Cache directory: /var/cache/pacman/pkg/ removing old packages from cache... done. Database directory: /var/lib/pacman/ Database directory cleaned up Note the only long operation is the "removing old packages from cache..." which notifies the user what is being done anyway. So I think it best not to output those messages. And if you make the "r" in "removing" a capital like every other message in that output at the same time, that would be good. Allan
On Wed, May 20, 2009 at 2:24 PM, Allan McRae
I'd be in favour of the less informative route. I.e.
sudo pacman -Sc --noconfirm Cache directory: /var/cache/pacman/pkg/ removing old packages from cache... done. Database directory: /var/lib/pacman/ Database directory cleaned up
Note the only long operation is the "removing old packages from cache..." which notifies the user what is being done anyway. So I think it best not to output those messages. And if you make the "r" in "removing" a capital like every other message in that output at the same time, that would be good.
In this particular case, it is fine indeed. But we have this generic question function. We could skip any printing there in case of --noconfirm, but then we would need to check that all the questions pacman can ask can be silenced.
Xavier wrote:
But we have this generic question function. We could skip any printing there in case of --noconfirm, but then we would need to check that all the questions pacman can ask can be silenced. Using --noconfirm implies a non-interactive session - which is how I noticed this. If a user wants to make use of --noconfirm then they must deal with the consequences if the default value isn't to their liking. There is already no way for the user to give a confirmation/rejection if noconfirm is set which is the correct behaviour. From my understanding, most defaults are safe defaults anyway.
question() has a default return value as a parameter, "short preset", and thus *always* has a default. Its also only ever called by yesno() - [Y/n], or noyes() - [y/N], both which are called appropriately throughout pacman. If there is a case which shouldn't have a default then the function is yet to be defined to handle that case. After a lot of grepping I believe that in every case where a question is currently displayed, there is subsequent output indicating what action then takes place. As per Allan's suggestion, I'm now more in favour of not printing the question at all. If we don't go that route then I'd still prefer adding the "... defaulting to ..." text rather than doing nothing. I have diffs from git ready for both cases - which shall I submit? Both? Do I just mail it here? -- __________ Brendan Hide
participants (3)
-
Allan McRae
-
Brendan Hide
-
Xavier