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

Add alt-shim support (fixes #3634) #3998

Merged
merged 7 commits into from Oct 23, 2020

Conversation

rasa
Copy link
Member

@rasa rasa commented May 25, 2020

Tested and working locally with both 71 and kiennq shims. See #3634 for details.
Hopefully:
Fixes #3294: Killing the shim process does not kill the child process,
Fixes #2921: Wait for console applications only,
Fixes #2339: python in cmd Ctrl+C problem,
Fixes #2006: Windows cmd gvim.exe shim waits for subprocess,
Fixes #1896: Handle Control-C in shim wrapper,
Fixes #1606: Support shims to both console and GUI apps,
Fixes ScoopInstaller/Extras#2801 : Putty shims open a blank window,

@rasa
Copy link
Member Author

rasa commented May 26, 2020

Testing instructions are here.

@rasa
Copy link
Member Author

rasa commented Jun 8, 2020

Can those that have tested this PR, please report your success/failure here. I tested both alternate shims and both worked fine.

@apommel
Copy link

apommel commented Jun 27, 2020

I tested them too and it is definitely an improvement. I did not encounter any issues, and the console flickering when launching a GUI from Win+R is gone. Keyboard interrupts also work correctly.

@rasa
Copy link
Member Author

rasa commented Jun 30, 2020

If there are no objections, I will merge this into develop

@chawyehsu
Copy link
Member

@rasa I suggest releasing previous commits in develop branch into the master branch before merging this (https://github.com/lukesampson/scoop/compare/develop).

@benferse
Copy link

A little late to the party, but I've been using this for a little over a week with no issues. Keyboard interrupts work as expected everywhere, which was my biggest problem.

lib/core.ps1 Outdated
switch ($shim_version) {
'71' { $shim_path = "$(versiondir 'scoop' 'current')\supporting\shims\71\shim.exe" }
'kiennq' { $shim_path = "$(versiondir 'scoop' 'current')\supporting\shims\kiennq\shim.exe" }
'default' { true }
Copy link
Contributor

@kiennq kiennq Aug 4, 2020

Choose a reason for hiding this comment

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

shouldn't this $true instead?
Or nothing should be put here.

Suggested change
'default' { true }
'default' {}

@trajano
Copy link

trajano commented Aug 5, 2020

Do you have instructions on how to enable this on my local scoop installation?

@MikeVensel
Copy link

The issue with ctrl + c killing PowerShell Core instances is very frustrating in my day to day use.

Is there any kind of timeline on when this will be merged and available?

@Ash258 Ash258 mentioned this pull request Aug 23, 2020
21 tasks
@alkuzad
Copy link

alkuzad commented Oct 14, 2020

This is mandatory to merge to continue recommend scoop to people, what are the problems that stop it from being added to master ? Branch/Repo shenanigans are not interesting to me.

@rasa
Copy link
Member Author

rasa commented Oct 22, 2020

OK, based on the feedback above, I'd will do the following after freshening this PR with the fix above, kiennq's latest release (2.2.1), and a little more testing:

  1. Merge the current develop into master
  2. Merge this PR into develop
  3. Merge the updated develop into master

I feel confident this will not introduce any issues, but if it does, we can always roll back, if needed.

@rasa rasa merged commit 7db0fe9 into ScoopInstaller:develop Oct 23, 2020
@woacademy
Copy link

@rasa when will this make it to master?

@rasa
Copy link
Member Author

rasa commented Nov 26, 2020

OK, let's do it now. We can always roll back if there are issues.

@rasa
Copy link
Member Author

rasa commented Nov 26, 2020

OK, develop has been merged into master. I also tagged the release at 2020-11-26.

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.

None yet

9 participants