Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Console] Fix backslash escaping in bash completion#43665

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:5.4fromGromNaN:console/5.4-complete-backslash
Oct 30, 2021

Conversation

@GromNaN
Copy link
Member

@GromNaNGromNaN commentedOct 22, 2021•
edited
Loading

QA
Branch?5.4
Bug fix?yes
New feature?no
Deprecations?no
Tickets#43664
LicenseMIT
Doc PR-

Fully tested code with#43598

  • printf options%b and%q are handy to escape string using in shell input.
  • Backslashes must to be escaped (doubled) in normal input (bin/console debug:form App\\Form\\CommentType). Otherwise they are dropped when PHP receive the argument (App\Form\CommentType becomeAppFormCommentType in$SERVER['argv']). This is not necessary for quoted arguments (bin/console debug:form "App\Form\CommentType" is OK). In the context of a command stored in a variable, like in the bash script, escaping is not interpreted: we must replace\\ by\ usingprintf '%b'.
  • Completion does not process escaping. IfApp\\Form\\ is typed, the suggestions must contain a\\ to match. Since these values are provided to shell, double backslash must be escaped:\ becomes\\\\ in suggestion output.
  • I choose to detect when quotes are typed to allow them in completion and add quotes to suggestions.
  • Suggestion with spaces are still not properly working.
screen-autocomplete-backslash.mov

@wouterj

This comment has been minimized.

@wouterj
Copy link
Member

sorry, I've hidden my comment. The PR changed a lot from what I looked at yesterday night. I'll have to look again before sharing my opinion

@GromNaN
Copy link
MemberAuthor

I'm rewriting this solution to use more native bash and less PHP. Mainly to remove dependency of the input in BashOutput.

@GromNaN
Copy link
MemberAuthor

@wouterj I'm done with something more bash-native. The only limitation is with some values that are not unescaped: the tokentwo\ words is not converted totwo words in completion input.

It would be awesome if we find a way to add tests to the bash script (unit or functional).

@GromNaNGromNaNforce-pushed theconsole/5.4-complete-backslash branch 3 times, most recently from4befcb8 tob914980CompareOctober 25, 2021 15:10
@stof
Copy link
Member

It would be awesome if we find a way to add tests to the bash script (unit or functional).

I agree with that, but I have no idea how.

wouterj reacted with thumbs up emoji

@wouterj
Copy link
Member

wouterj commentedOct 28, 2021•
edited
Loading

Hi!

I just tested it and I think this is perfect now you support both ways! I have 2 suggestions after quickly testing this:

  1. What do you think about favoring quoted values with special chars (especially\ or)? For example:
    $ bin/console debug:form# would become:$ bin/console debug:form 'Symfony\Component\Form\Extension\Core\Type\# instead of:$ bin/console debug:form Symfony\\Component\\Form\\Extension\\Core\\Type\\
  2. There is an error when the value only contains a quote:
    $ bin/console debug:form '[critical] Error thrown while running command "_complete -sbash -c2 -S5.4.0-DEV -ibin/console -idebug:form -i". Message: "The "--input" option requires a value."                             The "--input" option requires a value.                                              _complete [-s|--shell SHELL] [-i|--input INPUT] [-c|--current CURRENT] [-S|--symfony SYMFONY]
GromNaN reacted with thumbs up emoji

@fabpot
Copy link
Member

@GromNaN Do you have time to have a look at Wouter's comments? I'd love to be able to merge this one to unlock some other PRs. Thank you.

@wouterj
Copy link
Member

Imho, we can also merge this one and fix both of them between beta1 and rc1. The feature is complete - it's just some minor polishing.

@GromNaN
Copy link
MemberAuthor

I'm going to make the requested changes tonight.

@GromNaN
Copy link
MemberAuthor

What do you think about favoring quoted values with special chars

By default, completion of a FQCN will contain only a single quote, since namespaces are different. Not a big deal.

@GromNaNGromNaNforce-pushed theconsole/5.4-complete-backslash branch fromb914980 to56a74edCompareOctober 29, 2021 20:22
@GromNaNGromNaNforce-pushed theconsole/5.4-complete-backslash branch from5d29751 to7220038CompareOctober 29, 2021 20:39
@fabpotfabpotforce-pushed theconsole/5.4-complete-backslash branch from7220038 toe9e0c07CompareOctober 30, 2021 08:33
@fabpot
Copy link
Member

Thank you@GromNaN.

@fabpotfabpot merged commita3c6cca intosymfony:5.4Oct 30, 2021
fabpot added a commit that referenced this pull requestOct 31, 2021
This PR was merged into the 5.4 branch.Discussion----------[Console] [AppVeyor] Fix EOL in the tests| Q             | A| ------------- | ---| Branch?       | 5.4  <!-- see below -->| Bug fix?      | yes| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| License       | MIT[Probably caused by this fix](#43665)AppVeyor run tests on Windows and tests with EOL [failing](https://ci.appveyor.com/project/fabpot/symfony/builds/41346183#L1559)![image](https://user-images.githubusercontent.com/57155526/139556426-38a1b371-8c23-4211-a0cd-c31dc26034a7.png)`@derrabus` pls reviewCommits-------d4a9e6e [Console][AppVeyor] Fix EOL in the tests
@GromNaNGromNaN deleted the console/5.4-complete-backslash branchNovember 2, 2021 10:13
nicolas-grekas added a commit that referenced this pull requestSep 13, 2025
This PR was merged into the 6.4 branch.Discussion----------[Console] Fix handling of `\E` in Bash completion| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        | -| License       | MITRelated to#43665The problem with `$(printf -- '%b' "$w")` is that it also interprets `\E` as an escape sequence.For example, in```bashbin/console debug:form 'Symfony\Component\Form\Extension\Core\Type\EmailType'```the `\E` in the class name is replaced with the ASCII escape character:```Symfony\Component\Form�xtension\Core\Type�mailType```This PR switches to simply replacing double backslashes with a single one instead of using `printf -- '%b'`.Commits-------f1f50e6 [Console] Fix handling of `\E` in Bash completion
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@srozesrozeAwaiting requested review from sroze

@wouterjwouterjAwaiting requested review from wouterj

@xabbuhxabbuhAwaiting requested review from xabbuh

@ycerutoycerutoAwaiting requested review from yceruto

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

5 participants

@GromNaN@wouterj@stof@fabpot@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp