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

Closing braces are hidden inside folded regions #2133

Open
manojeeva opened this issue Nov 28, 2019 · 5 comments
Open

Closing braces are hidden inside folded regions #2133

manojeeva opened this issue Nov 28, 2019 · 5 comments
Labels
blocked on vs code / lsp / dap Requires a change in VS Code to progress in editor Relates to code editing or language features is enhancement
Milestone

Comments

@manojeeva
Copy link

Issue Type: Bug

Closing Tag does have extra intent and that leads to an issue When collapse it does not show the closing tag.

Before the Collapse and it is formatted.
Screen Shot 2019-11-28 at 13 16 41

After the Collapse

Screen Shot 2019-11-28 at 13 16 56

Extension version: 3.6.0
VS Code version: Code 1.40.2 (f359dd69833dd8800b54d458f6d37ab7c78df520, 2019-11-25T14:52:45.129Z)
OS version: Darwin x64 19.0.0

System Info
Item Value
CPUs Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz (4 x 2900)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off
surface_control: disabled_off
surface_synchronization: enabled_on
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 2
Memory (System) 8.00GB (0.31GB free)
Process Argv -psn_0_57358
Screen Reader no
VM 0%
@DanTup
Copy link
Member

DanTup commented Nov 28, 2019

When collapse it does not show the closing tag

Do you mean the closing } is "inside" the collapsed range instead of outside? I don't think is related to the indenting, I think this is always the case.

VS Code doesn't support folding partial lines with columns, only lines. This means we have to decide whether to include the last line or not inside the range. If we include it, then it's not rendered when collapsed (as shown here), and if we don't include it then the collapsed methods would have an additional line for just the closing brace.

Eg. What I'd like to have is:

myFunction() { [...] }

But actually we only have:

myFunction() { [...]

or:

myFunction() { [...]
}

I'm not really sure if the bottom one is a great improvement over the current one, since it takes more space in the editor just for the closing brace.

@DanTup DanTup changed the title Formatting issue with currying function Closing braces are hidden inside folded regions Nov 28, 2019
@DanTup
Copy link
Member

DanTup commented Nov 28, 2019

There are some related VS Code issues about this here:

microsoft/vscode#3352 (comment)
microsoft/vscode#70794

@DanTup DanTup added blocked on vs code / lsp / dap Requires a change in VS Code to progress in editor Relates to code editing or language features is enhancement labels Nov 28, 2019
@DanTup DanTup added this to the Backlog milestone Nov 28, 2019
@manojeeva
Copy link
Author

manojeeva commented Nov 28, 2019

When collapse it does not show the closing tag

Do you mean the closing } is "inside" the collapsed range instead of outside? I don't think is related to the indenting, I think this is always the case.

Yes, the closing brace is not showing only for the above syntax.

myFunction() {[...]
}

This one is okay like other but specifically for this syntax it is not showing closing brace

void somefunction()=>(){
[...]
}

The same is done in RN (ECMA). it shows the closing brace. see below

Screen Shot 2019-11-28 at 15 49 44

@DanTup
Copy link
Member

DanTup commented Nov 28, 2019

In my testing, these are all consistent - none of them show the closing brace. Here's a standard function:

Screenshot 2019-11-28 at 12 43 49 pm

I don't think what we have now is perfect, but doing it how I'd like requires a VS Code change. I do think it's better without the closing brace than with if it can't go on the same line though - it seems a waste of screen spaces to take up a whole line just to show a } that can be inferred. I think there is no value in showing line 634 in your RN sample.

@DanTup
Copy link
Member

DanTup commented Dec 8, 2020

microsoft/vscode#50840 is the relevant VS Code issue for this. If VS Code gets character-level folding, we should be able to have the } appear on the same line as the ... to avoid wasting lines with just closing brackets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked on vs code / lsp / dap Requires a change in VS Code to progress in editor Relates to code editing or language features is enhancement
Projects
None yet
Development

No branches or pull requests

2 participants