Skip to content
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(installed): If no $global, check both local and global installed #4798

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Mar 10, 2022

Description

Unintentional deletion in #4650 that breaks installed <app> (w/o $global) which check both locally and globally installed app.

Motivation and Context

Closes #4819

Relates to #4650

How Has This Been Tested?

Install lua as global app, before and after this fix:

image

Checklist:

  • I have read the Contributing Guide.
  • I have updated the documentation accordingly.
  • I have updated/passed the tests accordingly.

@hgkamath
Copy link

hgkamath commented Mar 22, 2022

your problem seems to me, is that you want the 2nd argument of the function installed to be boolean, when you are modifying its behavior to be tristate. so now you are relying on a code-introspection magic using PSparametersBound to determine the third state (false, true, not-given/implicit-default) and make it mean 'both'.

why not move away from boolean to enum
installed app local
installed app global
installed app both_local_global
enums could generalize to a long-term feature-add of "multiple global directories' , which I think is a nice feature, like for example I might want to put Firefox on SSD scoop install -g1 firefox and other less frequently used apps on HDD1 scoop install -g2 gimp or HDD2, if running out of space on HDD1, scoop install -g3 libreoffice-fresh . This may be better than multiple scoop installs and having to deal with changing SCOOP and SCOOP_GLOBAL environment variables

But I see this will require lot of work, if this call installed app $global is used everywhere

If you want to keep the boolean semantic, I think, rather than do magic inside the installed function, the following options are preferable :

  • It is less convoluted to have a new wrapper function installedbothLandG
  • or Don't bother with a wrapper function and just inline the 'OR-ed check' wherever required.
  • Another solution would be to never expect/allow user to execute update/reset/install on multiple installation directories together in a single command. i.e. implement scoop reset -g option. A user will then be expected to do scoop-reset-local and scoop-reset-global separately.

One does not expect a boolean argument to have 3 effects

just make sure this is well thought over and debated over

my 2 cents... I'm just a user, .. as long as it works

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.

3 participants