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

Allow leading spaces to bypass console aliases #13476

Merged
2 commits merged into from
Jul 11, 2022

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Jul 10, 2022

Summary of the Pull Request

When you create a console alias that overrides an existing command, it
should still be possible to execute the original command by prefixing it
with a space. However, at some point in the past, there was an attempt
to improve the usability by trimming leading spaces, and that ended up
breaking this functionality. This PR reverts that change, so leading
spaces can once again be used to bypass an alias.

PR Checklist

Validation Steps Performed

I've updated the existing alias unit test for leading spaces to match
the new behavior, i.e. it now confirms that a command with leading
spaces will not match the alias.

I've also manually confirmed that the doskey test case reported in
issue #4189 is now working as expected.

@ghost ghost added Area-CookedRead The cmd.exe COOKED_READ handling Area-Input Related to input processing (key presses, mouse, etc.) Impact-Compatibility Sure don't work like it used to. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase labels Jul 10, 2022
@j4james j4james marked this pull request as ready for review July 10, 2022 15:21
@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 11, 2022
@ghost
Copy link

ghost commented Jul 11, 2022

Hello @lhecker!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit cd2166a into microsoft:main Jul 11, 2022
@DHowett
Copy link
Member

DHowett commented Jul 11, 2022

Thanks for the fix!

DHowett pushed a commit that referenced this pull request Jul 19, 2022
## Summary of the Pull Request

When you create a console alias that overrides an existing command, it
should still be possible to execute the original command by prefixing it
with a space. However, at some point in the past, there was an attempt
to improve the usability by trimming leading spaces, and that ended up
breaking this functionality. This PR reverts that change, so leading
spaces can once again be used to bypass an alias.

## PR Checklist
* [x] Closes #4189
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #4189

## Validation Steps Performed

I've updated the existing alias unit test for leading spaces to match
the new behavior, i.e. it now confirms that a command with leading
spaces will not match the alias.

I've also manually confirmed that the `doskey` test case reported in
issue #4189 is now working as expected.

(cherry picked from commit cd2166a)
Service-Card-Id: 84072860
Service-Version: 1.15
@ghost
Copy link

ghost commented Aug 5, 2022

🎉Windows Terminal Preview v1.15.200 has been released which incorporates this pull request.:tada:

Handy links:

@j4james j4james deleted the fix-doskey-leading-space branch August 31, 2022 00:21
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Area-Input Related to input processing (key presses, mouse, etc.) AutoMerge Marked for automatic merge by the bot when requirements are met Impact-Compatibility Sure don't work like it used to. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't bypass DOSkey macro with leading space
5 participants