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

Change url() scope to avoid conflict with global aliases (e.g. Pansies) #4342

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

lediur
Copy link
Contributor

@lediur lediur commented Apr 28, 2021

Pansies, a popular PowerShell module, creates an alias from url to its New-Hyperlink function. This conflicts with usages of the url function in ./lib/manifest.ps1.

This PR specifies the scopes of those usages to the script level so those usages can find the function in the scoop library.

Fixes #4185

@lediur
Copy link
Contributor Author

lediur commented Apr 28, 2021

Seems like unrelated linting issues?

@Lockszmith-GH
Copy link
Contributor

This is an important fix. I'm guessing maintainers are overwhelmed with PRs.
Hope this PR will be accepted.

@Lockszmith-GH
Copy link
Contributor

@lediur It looks like the AppVeyor issue was resolved in June (#4027), after you've entered the PR. I don't know what's your option next - if there is a refresh option, I would recommend that, otherwise, maybe close this and send a new PR?

@lediur
Copy link
Contributor Author

lediur commented Oct 25, 2021

@lediur It looks like the AppVeyor issue was resolved in June (#4027), after you've entered the PR. I don't know what's your option next - if there is a refresh option, I would recommend that, otherwise, maybe close this and send a new PR?

Hey, thanks for the heads up! Let me try rebasing to retrigger the CI pipeline

edit: seems like there haven't been any commits to the main branch since I made this PR, I'll have to make a dummy commit

@lediur
Copy link
Contributor Author

lediur commented Oct 30, 2021

Since the repo has more maintainers maybe we can get this merged? Without this change, Scoop is broken for people who use Pansies.

@rasa @rashil2000 tagging some folks who may be able to move this along. Thanks for taking up maintenance of this tool! If there's anyone who is better able to move this along, please feel free to mention them.

@rashil2000 rashil2000 merged commit 35b2a42 into ScoopInstaller:master Nov 1, 2021
@lediur lediur deleted the pansies-compat branch November 1, 2021 03:36
@lediur
Copy link
Contributor Author

lediur commented Nov 1, 2021

Thanks @rashil2000 @niheaven!

@Lockszmith-GH
Copy link
Contributor

😞 just tested it, scoop-update.sp1, line 206 calls url and not script:url.
@rashil2000 @niheaven should fixing this be part of a new PR?

@rashil2000
Copy link
Member

Please make a new PR

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.

Scoop incompatible with Pansies module
4 participants