winget-cli icon indicating copy to clipboard operation
winget-cli copied to clipboard

Add prompts when installer aborts terminal or needs install location

Open florelis opened this issue 4 years ago • 3 comments

Resolves: #713 Resolves: #970

Changes:

  • Added user setting for default install root
  • Added prompts when an installer has InstallerAbortsTerminal (to confirm that we want to install) or InstallLocationRequired (to provide the install location).
  • Refactored the existing package agreements prompt to a common setup also used by the two new prompts. Before installing a package, we walk through all the possible prompts and show them if applicable. When installing multiple packages, we check all the packages that need the prompt and then show the prompt once.

Remaining items:

  • Testing
  • Use install root from settings
  • Update settings doc
  • Move installers that abort the terminal to the end on upgrade all/import
  • Skip prompts when running non-interactively
Microsoft Reviewers: Open in CodeFlow

florelis avatar Dec 18 '21 02:12 florelis

@check-spelling-bot Report

Unrecognized words, please review:

  • overriden
  • promptint
Previously acknowledged words that are now absent activatable amd Archs dsc Globals hackathon mytool Packagedx parametermap whatif
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the [email protected]:lechacon/winget-cli.git repository on the prompts branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/winget-cli/issues/comments/1020760372" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u

github-actions[bot] avatar Jan 25 '22 02:01 github-actions[bot]

Note that this is still missing tests, validation of the install root and the arg/setting to disable prompts. (Hence still a draft)

I had thought of moving installers that abort the terminal to the end so as to minimize their impact, but that seems tricky with dependencies.

florelis avatar Jan 25 '22 03:01 florelis

Reviving this old PR...

Main changes from last time:

  • Rebased on recent main branch
  • Added a setting and a CLI arg to prevent all prompts
  • Added unit tests

florelis avatar Jul 28 '22 19:07 florelis

This PR is having issues with the COM InProc E2E tests because I added something that causes us to load the string resources in a place we weren't doing it before, and the tests fail there because they cannot find the resources. I still don't know how to fix it (and it would make this change bigger), so I'm sidestepping it for now by preventing this new use of the ResourceLoader. I'll open an issue for actually fixing it because I imagine it will hit us again later.

florelis avatar Aug 11 '22 22:08 florelis