Skip to content

Removed [A]ll - yes to all #2849

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

Merged
merged 1 commit into from
May 29, 2025
Merged

Removed [A]ll - yes to all #2849

merged 1 commit into from
May 29, 2025

Conversation

lemonlambda
Copy link
Contributor

@lemonlambda lemonlambda commented Oct 6, 2022

Description Of Changes

Changed the prompt of Do you want to run this script?([Y]es/[A]ll - yes to all/[N]o/[P]rint) to Do you want to run this script?([Y]es/[A]ll/[N]o/[P]rint)

Motivation and Context

Unnecessary info as the command line app should assume the user knows what [A]ll, and this feature already exists for every other modern package manager on linux.

Testing

No tests have been performed.

Change Types Made

  • Bug fix (non-breaking change)
  • Feature / Enhancement (non-breaking change)
  • Breaking change (fix or feature that could cause existing functionality to change)
  • PowerShell code changes.

Related Issue

Resolves #2848

Change Checklist

  • Requires a change to the documentation
  • Documentation has been updated
  • Tests to cover my changes, have been added
  • All new and existing tests passed.
  • PowerShell v2 compatibility checked.

@CLAassistant
Copy link

CLAassistant commented Oct 6, 2022

CLA assistant check
All committers have signed the CLA.

@gep13
Copy link
Member

gep13 commented May 22, 2025

@lemonlambda thank you for raising this PR, and apologies for not coming back to you sooner about this.

We have been discussing this PR internally, and we have been trying to decide the best path forward. When you said...

and this feature already exists for every other modern package manager on linux.

Can you provide any concrete examples for where this is being done, and what the output looks like? We would love to be able to compare what Chocolatey CLI is currently doing, to other package managers, but we wanted to avoid having to go hunting, if you had an example at hand.

Apologies again about not coming back to you sooner here. As I have said a few times, "the wheels may turn slowly around here ...but they do turn" 😄

@lemonlambda
Copy link
Contributor Author

Can you provide any concrete examples for where this is being done, and what the output looks like? We would love to be able to compare what Chocolatey CLI is currently doing, to other package managers, but we wanted to avoid having to go hunting, if you had an example at hand.

I can't show it from the terminal anymore so I've had to find it in code: https://github.com/Jguer/yay/blob/8ab365284651c99f598d6bc30868f30ec9488ce1/po/en.po#L40-L42

Apologies again about not coming back to you sooner here. As I have said a few times, "the wheels may turn slowly around here ...but they do turn" 😄

I understand :)

@pauby
Copy link
Member

pauby commented May 23, 2025

Given the example above, I think we should use Do you want to run this script?([Y]es/[A]ll scripts/[N]o/[P]rint).

What are your thoughts?

@gep13
Copy link
Member

gep13 commented May 27, 2025

@pauby said...
What are your thoughts?

No objections from me.

@lemonlambda as the original creator of this PR, would you have any objections to this compromise?

@lemonlambda
Copy link
Contributor Author

I think it's shorter and more clean so sounds good to me. Although I still prefer just [A]ll

@lemonlambda
Copy link
Contributor Author

I have updated the branch to be up to date with the most recent commits, and did the all scripts change

After some discussion with the Chocolatey team, it has been decided that
the prompt option for "All - yes to all" is rather verbose, and the need
to include "yes to all" is unneeded, and not in line with how other
package managers do it when prompted with similar choices.

This commit removes the "All - yes to all" and replaces it with simply
"All scripts", which is more succinct.  In addition, the unit/pester
tests have been updated to reflect this change.  It was necessary to
remove a unit test, since in the previous prompt "all - yes to all" was
actually treated as two options, split on the "-", i.e. both "all" and
"all - yes to all" were accepted answers to the prompt.  This is no
longer the case when the option is "All scripts", so only one unit test
is now required.
@gep13
Copy link
Member

gep13 commented May 29, 2025

@lemonlambda I have taken the liberty to rebase this PR and fix up the commit a little bit. I have fixed the unit tests, as there was a failing one due to the change made here, and I have updated the wording of the commit message to include information about what is being done here.

I have taken this for a spin this morning and things are working as expected, with the correct prompt answers being handled:

image

Once the CI builds complete, I will get this merged.

Thanks again for your contribution to Chocolatey CLI!

@gep13 gep13 enabled auto-merge May 29, 2025 07:28
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gep13 gep13 merged commit db1eb58 into chocolatey:develop May 29, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace verbose option for "[A]ll - yes to all" when installing packages
4 participants