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

Support auto-closing quotes in Python raw string literals, etc #35636

Merged

Conversation

Syeberman
Copy link

Python uses a "string prefix" to denote bytes literals (b''), raw strings (r''), and formatted string literals (f''). This change makes it so that when one types r' in VS Code, the string will be auto-closed as r'' (as an example).

Python also supports multiple string prefixes for a single literal. I've tested that typing fr' will auto-close to fr'', and so forth.

Python uses a ["string prefix"](https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals) to denote bytes literals (`b''`), raw strings (`r''`), and formatted string literals (`f''`).  This change makes it so that when one types `r'` in VS Code, the string will be auto-closed as `r''` (as an example).

Python also supports multiple string prefixes for a single literal.  I've tested that typing `fr'` will auto-close to `fr''`, and so forth.
@msftclas
Copy link

msftclas commented Oct 5, 2017

CLA assistant check
All CLA requirements met.

@chrmarti chrmarti added the languages-basic Basic language support issues label Oct 5, 2017
{ "open": "f'", "close": "'", "notIn": ["string", "comment"] },
{ "open": "F'", "close": "'", "notIn": ["string", "comment"] },
{ "open": "b'", "close": "'", "notIn": ["string", "comment"] },
{ "open": "B'", "close": "'", "notIn": ["string", "comment"] }
Copy link

Choose a reason for hiding this comment

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

It may be unit tested.

@aeschli
Copy link
Contributor

aeschli commented Oct 6, 2017

Isn't it enough to just have ' and " as an auto pair character? The only difference I'm aware of is that undo will only remove the quote, instead of the letter before, but IMO that's better anyway.

@Syeberman
Copy link
Author

The original auto-close setting did not auto-close r', etc. Presumably, it's coded to only trigger after a whitespace (or non-word boundary), but I didn't look at the code.

Oddly, this doesn't explain why the above configuration auto-closes fr', etc. If the open pattern must be preceded by whitespace, then that shouldn't work. Perhaps it's coded differently if the pattern starts with an alphanumeric character?

@aeschli
Copy link
Contributor

aeschli commented Oct 12, 2017

Yes, there's a condition to not autoclose ' after a word.
@alexandrudima Do you want to reconsider that, or is the PR ok?

@aeschli aeschli added this to the October 2017 milestone Oct 12, 2017
@aeschli aeschli added the feature-request Request for new features or functionality label Oct 12, 2017
@aeschli aeschli modified the milestones: October 2017, November 2017 Nov 3, 2017
@aeschli
Copy link
Contributor

aeschli commented Nov 3, 2017

Sorry, this slipped. @alexandrudima Waiting for your opinion.

@alexdima
Copy link
Member

alexdima commented Nov 14, 2017

The following added rules do nothing:

		{ "open": "r\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "R\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "u\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "U\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "f\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "F\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "b\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "B\"", "close": "\"", "notIn": ["string", "comment"] },
		{ "open": "r'", "close": "'", "notIn": ["string", "comment"] },
		{ "open": "R'", "close": "'", "notIn": ["string", "comment"] },
		{ "open": "u'", "close": "'", "notIn": ["string", "comment"] },
		{ "open": "U'", "close": "'", "notIn": ["string", "comment"] },
		{ "open": "f'", "close": "'", "notIn": ["string", "comment"] },
		{ "open": "F'", "close": "'", "notIn": ["string", "comment"] },
		{ "open": "b'", "close": "'", "notIn": ["string", "comment"] },
		{ "open": "B'", "close": "'", "notIn": ["string", "comment"] }

Out of the added rules, this is the only one that might do something:

		{ "open": "'", "close": "'", "notIn": ["string", "comment"] },

Closing Pairs are implemented to be working on single characters. Basically, the code evaluates if someone typed the character in the open part of a closing pair and does not consider any characters before the auto closing open character.

@Syeberman
Copy link
Author

@alexandrudima I tested this configuration myself when I submitted this PR. Without my changes, when you type r', it does not auto-close to r''. With my changes, it properly auto-closes raw, unicode, f-strings, and bytes.

@alexdima
Copy link
Member

Yes, that's what I'm saying too. The only line that makes this work from this PR is:

{ "open": "'", "close": "'", "notIn": ["string", "comment"] },

This defines that ' - ' forms an auto-closing pair.

The other 16 lines serve no purpose.

@aeschli
Copy link
Contributor

aeschli commented Nov 15, 2017

@alexandrudima
{ "open": "'", "close": "'", "notIn": ["string", "comment"] }, is already part of the autoClosingPair.
It doesn't work after r' due to this.
Adding { "open": "r'", "close": "'", "notIn": ["string", "comment"] }, fixes the problem

@alexdima
Copy link
Member

@Syeberman @aeschli I agree the change makes this feature work. I was quite stumped at first, since I am pretty sure how auto-closing characters should work and how they're implemented.

As I thought, the execution does not enter the auto closing open character code (since that is implemented to only work on single characters), but it enters BracketElectricCharacterSupport._onElectricAutoClose and these newly added auto-closing pairs somehow make their way to _complexAutoClosePairs.

@aeschli Since this is something you've authored in ebc3074 , I cannot be of further assistance.

Perhaps now would be a good time to document somewhere this behaviour!

@aeschli aeschli merged commit d4d3d40 into microsoft:master Nov 30, 2017
@aeschli
Copy link
Contributor

aeschli commented Nov 30, 2017

Talked to @alexandrudima again and we are fine with the PR. Not autoclosing ' and " after a letter is the expected behaviour most languages, so the PR makes sense to add additional rules fo the cases where this makes sense.

Thanks @Syeberman!

@redeemefy
Copy link

I still have the issue on Version 1.21.1 (1.21.1) and running OSX Sierra.

@Syeberman Syeberman deleted the python_auto_closing_string_prefix branch March 23, 2018 13:07
@aeschli
Copy link
Contributor

aeschli commented Mar 26, 2018

@diazgilberto Please file a new issue with a code sample that shows the problem. Also try to run without extensions to see whether an external extension causes this.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality languages-basic Basic language support issues verification-needed Verification of issue is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants