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

Incorrect indentation after string literals containing escaped characters #4241

Closed
caudate-julie opened this issue Jan 31, 2019 · 5 comments
Closed
Assignees
Labels
area-formatting bug Issue identified by VS Code Team member as probable bug

Comments

@caudate-julie
Copy link

Steps to reproduce:

  1. Type string with '\n' in it, press Enter.
a = '\n'

Same for double quotes.

Expected behaviour

a = '\n'
<--- cursor should be here

Actual behaviour

a = '\n'
    <--- cursor is here

Environment data

  • VS Code version: 1.30.2
  • Extension version (available under the Extensions sidebar): 2019.1.0
  • OS and version: Windows 10
  • Python version (& distribution if applicable, e.g. Anaconda): 3.7
  • Type of virtual environment used: N/A
  • Relevant/affected Python packages and their versions: none

Logs

Console:

Starting Jedi Python language engine.

Linter 'pylint' is not installed. Please install it or select another linter".
Error: Module 'pylint' not installed.
@ghost ghost added the triage-needed Needs assignment to the proper sub-team label Jan 31, 2019
@caudate-julie
Copy link
Author

I am not familiar with the code, but it may be this string:
https://github.com/Microsoft/vscode-python/blob/c23603c9b7a3dbb805e8339a2d7cf01bb7327b34/src/client/extension.ts#L176
since regex doesn't include end of line.

@caudate-julie caudate-julie changed the title Incorrect indentation after string literals containing newline Incorrect indentation after string literals containing escaped characters Jan 31, 2019
@d3r3kk d3r3kk added the triage label Jan 31, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Jan 31, 2019
@DonJayamanne DonJayamanne added bug Issue identified by VS Code Team member as probable bug needs PR labels Feb 4, 2019
@DonJayamanne DonJayamanne removed their assignment Feb 4, 2019
@jkyeung
Copy link

jkyeung commented Mar 9, 2019

Yeah, this is really annoying. It seems like this was introduced by the "fix" for #3284.

If I had to choose between having line continuation backslashes work properly, or backslash escapes work properly, I would choose backslash escapes in a heartbeat. Some people probably never use backslash for line continuation, while most other people probably use it very sparingly, since PEP 8 encourages implicit line continuation wherever it can be sensibly applied.

And even if you like backslash line continuation, (1) presumably you're not going to have large sections of code riddled with them, since long lines are discouraged anyway, and (2) you have reasonable alternatives for achieving what you need. Whereas there are plenty of situations where you could be using a lot of backslash escapes (strings, regular expressions), and there is no reasonable alternative.

@jkyeung
Copy link

jkyeung commented Jun 12, 2019

I am not familiar with the code, but it may be this string:
https://github.com/Microsoft/vscode-python/blob/c23603c9b7a3dbb805e8339a2d7cf01bb7327b34/src/client/extension.ts#L176
since regex doesn't include end of line.

I'm pretty sure you've hit the nail on the head.

It seems like such a simple and easy fix (see my comment on #5821, which perhaps should gave gone on #3915). If we are wrong about this, and it's actually more complicated, then I wish they would come out and explain why.

@karrtikr
Copy link

karrtikr commented Jun 26, 2019

Prescribed Solution:

There may be a more correct regex to use:

/^(?!\s+\\)[^#\n]+\\$/

See #5821 (comment).

Make sure this does not break the following (from #3284):

a = 1 + \
    <--- indent is here

@ericsnowcurrently
Copy link
Member

I've verified that the following indent/dedent properly:

a = '\n'
# cursor here

a = 1 + \
    # cursor here

@ghost ghost removed the needs PR label Jul 9, 2019
@ericsnowcurrently ericsnowcurrently removed their assignment Jul 9, 2019
kimadeline added a commit that referenced this issue Jul 12, 2019
* Dedent on elif|else
* Add unit tests (will conflict with #4241)
* More descriptive comment (same as f5daf78)
* Move tests from extension to language config file
* Add news file
* Use regex we just introduced
* Space is free
* Update regexes and existing tests (so they pass)
* Add support for open tuples and dictionaries
* Shuffle and add tests
* Typo + more descriptive key name
* Updated news file after new changes
* Apply suggestions from code review (first batch)
Co-Authored-By: Eric Snow <ericsnowcurrently@gmail.com>
* Update src/client/language/languageConfiguration.ts
Co-Authored-By: Eric Snow <ericsnowcurrently@gmail.com>
* Third batch of review fixes
* Add some comments
* Separate unit tests
* More test simplification
* Simplify return regex
* Remove unnecessary tslint disable rule
* pipeline is stuck
@lock lock bot locked as resolved and limited conversation to collaborators Jul 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-formatting bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

No branches or pull requests

8 participants