Add option preserve_trailing_whitespace#241
Conversation
This implements GDQuest#237. We encode trailing whitespace (tabs and spaces) to hide them from Topiary to prevent removal of them. Then we call Topiary as usual. Finally, we restore the encoded whitespace to their original form.
|
In general, the approach for preserving and restoring trailing white space would be the one you've followed in this PR, thank you very much for your work so far. Before you address the issue of some instructions lines losing their indentation, I left a question on #237 to verify that the problem is really about trailing whitespace vs actually just preserving tabs on empty lines within a code block, which would be a simpler thing, i.e. feature request #151. |
|
As we got confirmation that the wanted feature is actually preserving leading tabs, I would recommend adjusting this to work in one of two ways:
Either way, we should probably rename the option to something like "preserve indenttation on empty lines" I generally have slight preference for the AST WOG. Though it's not very important because A long-term plan is to rewrite the formula in C++. Without relying on this Topiary library. And this code will likely change with the re-implementation. So the topiary-based/rust version will serve as a reference for formatting output and the rewrite allow us to work toward tooling that can be more long-lived and possibly be integrated into Godot. A contributor offered to work on the C++ port in #230 |
|
I think, I'll leave this approach here, should the need arise to preserve all the whitespace. For #151 however, I think we'll need another approach - I'll discuss there. |
|
Thank you very much for your work. I should have thought about asking about the white space change before, but I think I won't merge this feature right now. I would wait for this to be requested/a clear need for multiple people. Just so you know, I use issues, notably feature request as a way for people to vote or give feedback on things and voice in, to discuss and assess the need. Because usually people only look at issues. Either directly open one or quickly check the existing ones and leave a reaction or a comment. |








Please check if the PR fulfills these requirements:
Related issue (if applicable): #237
What kind of change does this PR introduce?
This implements #237.
Workflow of the implementation:
Does this PR introduce a breaking change?
I don't think so.
There might arise some problem, when the editor plugin is newer than the formatter binary and the option get's checked in there, as the formatter will then get an command line option it does not understand. Updating the binary should fix this though.