search

LaurentTreguier / vscode-uncrustify

Code format using uncrustify
66 stars 10 forks source link

Consider actual filename for formatting the file. #60

Closed samishchandra closed 4 years ago

samishchandra commented 4 years ago

I am using this extension for a while and it's working great.

I am trying to add an option to Uncrustify to better sort import orders using PR 2710.

However the extension doesn't take filename into consideration when formatting. If the extension is using temporary file for formatting can it use the same name as that of the actual filename, so that the above functionality works fine. Thankyou

samishchandra commented 4 years ago

Used the following config as workaround.

"uncrustify.useReplaceOption": true,
LaurentTreguier commented 4 years ago

This extension sends the file content through stdin to uncrustify, that's why it doesn't sue the file name by default. And indeed, using "uncrustify.useReplaceOption": true fixes it by forcing VSCode to save the file, ans then formatting the file directly.

samishchandra commented 4 years ago

But setting this option doesn't work with option "formatOnSave" in VScode. ?

LaurentTreguier commented 4 years ago

Setting useReplaceOption to true and formatOnSave to true still works for me

samishchandra commented 4 years ago

It shows me this process dialog and never completes.

Screen Shot 2020-04-03 at 11 39 24 AM

But when I make useReplaceOption = false.. it works.. but doesn't take filename into account though.

LaurentTreguier commented 4 years ago

Does it also do this on files outside of Dropbox ?

LaurentTreguier commented 4 years ago

Nevermind, Dropbox syncs files on disk, it's not a remote filesystem.

LaurentTreguier commented 4 years ago

What are the contents of the test file ?

samishchandra commented 4 years ago

May be related issues ?

https://github.com/microsoft/vscode/issues/50123 https://github.com/microsoft/vscode/issues/50351 https://github.com/microsoft/vscode/issues/88149

samishchandra commented 4 years ago

If it helps with debugging.. clicking the "Cancel" button above actually outputs the extensions log and formats the file.. which is weird..

LaurentTreguier commented 4 years ago

I think the problem comes from the fact that when formatting, it will try to save the file, then saving will cause formatting, which will cause saving, etc. During formatting, without using useReplaceOption, I have to use the file content from VSCode without reading the file on the disk, and I send it to Uncrustify on stdin. There is no way to specify the filename to Uncrustify when sending data through stdin, as far as I know.

samishchandra commented 4 years ago

Oh interesting, is there a way to write contents of the file to temporary file with same filename.. and get results of the formatter ? Probably that way we avoid the loop.

LaurentTreguier commented 4 years ago

This could be possible, although not very good for performance

samishchandra commented 4 years ago

Can we support it with a option to control may be. Thanks.

LaurentTreguier commented 4 years ago

This should be possible using uncrustify.useTempFile now. Additionally, maybe it will also work with uncrustify.useReplaceOption, there was indeed a bug with it that should be fixed now

samishchandra commented 4 years ago

Thanks a lot Laurent for fixing the issue in quick time. Really appreciate it.

samishchandra commented 4 years ago

Is this available on the latest? or do I have to do something ?

LaurentTreguier commented 4 years ago

It should be available on the latest version

samishchandra commented 4 years ago

Oh, I think the version it is compatible is ^1.43.0.. but i am currently on vscode 1.42.0.. is it a hard restriction or can we lower that version ?

samishchandra commented 4 years ago

Btw.. i tested the flag by downloading latest vscode.. and its working as expected. It would be great, if you can lower the version check if possible. Thank you once again to make this possible.