-
-
Notifications
You must be signed in to change notification settings - Fork 22
First draft of new closeHangingIndent option #128
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
base: main
Are you sure you want to change the base?
Conversation
Based on manual testing, this change does what I want, except in one situation, and has one bug that I'm not sure how to fix.
From this:
```
some_long_function_name("test")
```
You get this just by pressing Enter in the right places:
```
some_long_function_name(
"test"
)
```
From this:
```
some_long_function_name({"logger": {"info": logger.info}})
```
You can get this from just Enter as well:
```
some_long_function_name(
{
"logger": {
"info": logger.info
}
}
)
```
However, from this:
```
some_long_function_name({
"WSGIErrorsWrapper": WSGIErrorsWrapper})
```
Pressing Enter before the } doesn't put the }) at the start of the line. And pressing Enter before the ) mysteriously puts it right under the "e" in Wrapper, even if you've already pressed Enter before the }. Not sure what to do about that.
Also, the complex hanging indent case mentioned in the comments in nextIndentationLevel no longer works as intended.
```
x = [
0, 1, 2, [3, 4, 5,
6, 7, 8],
]
```
Becomes this after pressing Enter between 8 and ] :
```
x = [
0, 1, 2, [3, 4, 5,
6, 7, 8
],
]
```
I imagine this is undesired, but I don't know how to accomplish my goal without breaking this. The code path for my two successes and this failure are the same (the first if block at the end of nextIndentationLevel()).
|
Thanks @coredumperror! I have a busy week this week, so I will try and review some time next week. If it's been a while and I haven't responded, feel free to remind me. It also sounds like you were able to set up a dev environment, so hopefully that means you can generate the VSIX file, which you can install manually to get the behavior you desire sooner. (I know the pain of using an editor that doesn't behave exactly how I want!) |
|
Yeah, I followed the guide from VS Code's docs for making a new extension, and then used what I learned from that to get Python Indent dev up and running. Had to install some node stuff to make the build work, but that wasn't too hard to figure out. Thanks for the heads up about VSIX. I hadn't heard of that, but I assume you mean what they talk about here? https://code.visualstudio.com/api/working-with-extensions/publishing-extension |
|
Yes, specifically the part under the header https://code.visualstudio.com/api/working-with-extensions/publishing-extension#packaging-extensions If you're able to run the 'vsce package' command that will create a file version of the extension that can be installed directly. |
|
Did you ever get a chance to look at this PR? I was just reminded of it because I had to re-install VS Code this week. |
|
Hey @coredumperror sorry for the super long delay. The changes look overall reasonable to me. I do have a few comments, though. Can you let me know if you have the bandwidth / will to look at them? |
|
Yeah sure, I would love to get this feature fully fleshed out and merged. |
PR for #127
Based on manual testing, this change does what I want, except in one situation, and has one bug that I'm not sure how to fix.
From this:
You get this just by pressing Enter in the right places:
From this:
You can get this from just Enter as well:
However, from this:
Pressing Enter before the }) gives you this:
Which I was hoping would be this:
And pressing Enter before the ) mysteriously puts it right under the "e" in Wrapper, even if you've already pressed Enter before the }.
Unfortunately, the complex hanging indent case mentioned in the comments inside
nextIndentationLevel()no longer works as intended.Becomes this after pressing Enter between 8 and ] :
When it's supposed to be this:
I'm not sure how to accomplish my goal without breaking this. The code path for my two successes and this failure are the same (the first
ifblock at the end ofnextIndentationLevel()).Checklist
I had to use optional arguments to move
dedentIfHangingaround within the code without breaking the tests. I have no prior experience with TypeScript, so I was pretty baffled about what to do to update them. I'll try to get that figured out next week.I'm also not 100% sure my description for the new setting is quite right.