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

Using ItemGroup, PropertyGroup (etc) suggestions from the extension results in incorrect indentation #93

Open
alexrp opened this issue Dec 10, 2021 · 18 comments

Comments

@alexrp
Copy link

alexrp commented Dec 10, 2021

Let's say I have a file like this:

<Project>
</Project>

If I start to type an item group like

<Project>
    <Item
</Project>

and accept the <ItemGroup> suggestion from the extension, I end up with:

<Project>
 <ItemGroup>
   
 </ItemGroup>

This is with indent_size = 2 in .editorconfig. With indent_size = 4, it instead ends up as:

<Project>
   <ItemGroup>
       
   </ItemGroup>
@tintoy
Copy link
Owner

tintoy commented Jan 1, 2022

Hi, sorry for the late reply - I didn't see the notification email for this until this morning :)

Just checking if I understand correctly: are you saying that it appears to be using the indent size specified in settings (in this case, from .editorconfig), rather than the indent level (i.e. column) where you started typing?

@alexrp
Copy link
Author

alexrp commented Jan 1, 2022

It seems to respect both the current indent level and indent_size from .editorconfig, but it's always off by one space.

@tintoy
Copy link
Owner

tintoy commented Jan 3, 2022

Sorry, I can see that'd be annoying. I'll look into it and let you know what I find 🙂

@RReverser
Copy link

This is still an issue. Did you ever get time to investigate it?

@tintoy
Copy link
Owner

tintoy commented Sep 24, 2024

Hi - sorry, I remember investigating, but I can’t remember what I found. I’ll have another look and see if there is anything obvious.

just as a quick check, does this happen when the file’s language is set to MSBuild as well as when it is set to XML?

@RReverser
Copy link

RReverser commented Sep 24, 2024

I don't think this extension's autocomplete even works when set to XML? At least I only tried with language set to MSBuild.

@tintoy
Copy link
Owner

tintoy commented Sep 24, 2024

Ok, thanks - that helps to cut down on the possibilities :)

@tintoy
Copy link
Owner

tintoy commented Sep 30, 2024

I wonder if this could be related to tintoy/msbuild-project-tools-server#71

@tintoy
Copy link
Owner

tintoy commented Oct 3, 2024

I’m still looking into this, but no luck so far. I suspect it maybe be related to how we compute the span of text to be replaced by the completion but it’s tricky to test for.

@RReverser
Copy link

This kind of off-by-one error almost seems like something is mixing up 1-based column numbers with 0-based ones.

@tintoy
Copy link
Owner

tintoy commented Oct 4, 2024

While it isn’t impossible, everything uses the same data structures to represent LSP line-and-column vs internal line-and-column (and the translations are always done by the same function), it so may not be all that likely (if that was broken it would be broken everywhere). Worth checking though, so I’ll do that, thanks 🙂

https://github.com/tintoy/msbuild-project-tools-server/blob/e942b1a1180cf8e47ff4aa9584d4adc4204d04f9/src/LanguageServer.Common/Position.cs#L296 (etc)

tintoy added a commit to tintoy/msbuild-project-tools-server that referenced this issue Oct 5, 2024
tintoy added a commit that referenced this issue Oct 5, 2024
@tintoy
Copy link
Owner

tintoy commented Oct 5, 2024

I think I've figured this out - it seems a while back, the behaviour changed when completion is triggered via trigger characters (in this case, the last character typed is implicitly treated as part of the current selection, if it triggered completion).

@tintoy
Copy link
Owner

tintoy commented Oct 19, 2024

Sorry for the delay, I’ll put together a new release shortly.

@tintoy
Copy link
Owner

tintoy commented Oct 20, 2024

Published v0.6.5 (it may take up to 15-20 minutes before the new version is visible in the VS extension gallery).

@RReverser - if you have a moment, would you be able to give the new version a try and see if it fixes the problem for you?

@RReverser
Copy link

Looks like there's a different issue now: I'm typing <, then selecting any of the autocomplete tags, e.g. Analyzer. When I press Enter, I'm getting <<Analyzer Include="" /> - that is, the original < is not replaced.

@tintoy
Copy link
Owner

tintoy commented Oct 23, 2024

Is your language set to XML or MSBuild?

@RReverser
Copy link

Oops, yeah, I changed them to XML while this extension's autocomplete was broken. Seems to work with MSBuild.

Why does the language affect such subtle detail?

@tintoy
Copy link
Owner

tintoy commented Oct 23, 2024

It’s because of the way VS Code defines delimiters for each language, I believe (its effects are subtle but definite). We don’t control the XML language definition, and they changed it a while back in some way that broke our completion behaviour and I’ve not found a consistent solution to it when using their definitions (every change I make to fix something breaks something else 😂). The way we define the XML language (as used in MSBuild project files) doesn’t have that problem.

I guess I could modify the language server so that this latest fix is only applied when the language is MSBuild (but not when it is XML)?

@DoctorKrolic, what do you reckon, worth a try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants