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

Handle whitespaces somehow #360

Open
sungam3r opened this issue Oct 16, 2023 · 3 comments
Open

Handle whitespaces somehow #360

sungam3r opened this issue Oct 16, 2023 · 3 comments

Comments

@sungam3r
Copy link
Member

sungam3r commented Oct 16, 2023

@my @your instead of @my @your now. It could be fixes as well checking parent node but for directives parent is always not null - it is Directives container, so actually grand parent is needed. Moreover, even after such check some tests failed (see Printer_Should_Print_Pretty_If_Directives_Skipped). The core problem is that some arbitrary nodes can be skipped so it's not obvious how to handle whitespaces at all. General approach may be to require each node to print their leading whitespace if it should do that. Downside - it will complicate all printing code. I do not want to do that just to handle very rare cases.

Originally posted by @sungam3r in #359 (comment)

@Shane32
Copy link
Member

Shane32 commented Oct 16, 2023

Have you thought about making an protected/private method which deduplicates whitespace by examining the leading/trailing character to remember if it's whitespace (space or CR/LF)? Then just strip leading space character if it's a duplicate (unless indenting)? No allocation when stripping whitespace because we are using ROMs.

This change could likely be made almost entirely within PrintContextExtensions.WriteAsync with an extra overload or method to support indenting. Minor changes would need to be made to WriteEncodedStringAsync and WriteMultilineBlockString also so that no whitespace is stripped from within a string.

@Shane32
Copy link
Member

Shane32 commented Oct 16, 2023

Taking this a step further, if we abstract all of the formatting away from the SDLPrinter into the print context, we could add an option to print 'minimized', handled entirely by the print context. It would just strip or ignore all whitespace except where needed, detected solely by the last character printed compared with the next character to print. So "d" and "(" do not require a space between them in "field (arg: 2)", and nor does ":" and "2". So the print context might have methods such as:

  • WriteToken (should not include spaces) (writes current indent if on a new line)
  • WriteString (formatting for GraphQL and encapsulating quotes are done in here)
  • WriteComment (adds #)
  • WriteBlockString
  • WriteLine
  • Indent (returns IDisposable struct??)
  • Unindent

@sungam3r
Copy link
Member Author

I thought a little about all this. In general, an additional state in context and management of that state will be required.

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

No branches or pull requests

2 participants