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

Fix multi line formatting #10780

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Conversation

maxRN
Copy link
Contributor

@maxRN maxRN commented Jul 29, 2024

No description provided.

@maxRN maxRN force-pushed the fix-multi-line-formatting branch from 9192d46 to 8862885 Compare July 29, 2024 17:34
maxRN added 10 commits July 31, 2024 15:49
Signed-off-by: Max Große <[email protected]>
Signed-off-by: Max Große <[email protected]>
Signed-off-by: Max Große <[email protected]>
Signed-off-by: Max Große <[email protected]>
@maxRN maxRN force-pushed the fix-multi-line-formatting branch from 5fefd7f to e7024d4 Compare July 31, 2024 13:49
Signed-off-by: Max Große <[email protected]>
@@ -5,6 +5,7 @@ open Stdune
type t =
| Atom of Loc.t * Atom.t
| Quoted_string of Loc.t * string
| Block_string of Loc.t * string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to introduce the Block_string only in the CST?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not without further work: Dune_sexp.parse parses all values into a list of Encoded.t which are actually Ast.t values and turns those back into Cst.t. So if we don't have a Block_string in the AST we lose precision so to speak, because we would turn a Cst.Block_string into a Ast.Quoted_string and then we can no longer know if it's really a Quoted_string or a Block_string.

Maybe we could somehow adjust the parsing function and add an option to directly return it as Cst.t values.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not change string by a richer abstract type that would remember whether it came from a quoted string or a blocked string, as well as having a to_string to get the underlying string. The type could also include the Loc.t.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this idea and will probably implement it like this, but there is more work to be done for block strings that are also templates.

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

Successfully merging this pull request may close these issues.

3 participants