Symbolic strings #994

matthew-healy · 2022-12-14T15:40:22Z

This PR introduces a simple form of "symbolic strings" (#948), whereby the syntax s%"..."% is just de-sugared to an array.

vkleen

This looks good! I think we should merge this PR and see if the semantics of symbolic strings need chaning after experimenting with them a bit. Similarly, the string delimiters can be bikeshed later.

yannham

Mostly LGTM. I just wonder what we should do about the "multiline" semantics of symbolic strings. The first question is: should we have symbolic strings be multiline by default, or offer a separate version for multiline strings?

Another solution would be to have s"foo" strings as well. I think C++ has those. Then there is a clear delimitation between the multiline axis and the "special" axis.

The second question, which is independent, is what to do with interpolated blocks inside mutliline strings. Currently, we carry around inside string chunks an extra ident arguments to handle those in the natural way (see #994 (comment)). One possibility would be to pass this argument to the parsing function, but it can quickly become cumbersome to pass this information around.

Another solution is just to abandon this semantics for symbolic strings (and do not interpolate other strings as "blocks"). It would be inconsistent though, that the behavior differs between multiline strings and those symbolic string.

yannham · 2022-12-15T14:27:35Z

src/parser/grammar.lalrpop

@@ -490,26 +495,35 @@ StrChunks: RichTerm = {
            .chain(lasts.into_iter())
            .collect();

-        let mut chunks = if start == StringKind::Multiline {
+        let chunks = if start.needs_strip_indent() {


I had this in mind but forget to ask. Should we behave like multiline string with respect to indentation, or not? It seems like you answer "no", which is very reasonable. That being said, I wonder if we then would like to combine the semantics of symbolic strings and multiline strings, e.g. in Nix often we currently use multiline strings to write builder commands, where symbolic strings would be used. This may not be trivial with respect to indenting interpolated values, though. I guess we don't have to answer this question right now.

src/parser/utils.rs

yannham · 2022-12-15T14:49:25Z

src/parser/utils.rs

+    pub fn needs_strip_indent(&self) -> bool {
+        match self {
+            StringStartDelimiter::Standard => false,
+            StringStartDelimiter::Multiline | StringStartDelimiter::Symbolic => true,


Ah, so what I wrote before is wrong, multiline strings and symbolic strings actually behave the same. I'm not sure how this works with interpolated values though. There is some logic at runtime such that:

let bar = "alpha\nbeta" in m%" foo %{bar} baz "%

evaluates to

foo alpha beta baz

I don't think we can easily keep this logic for symbolic strings, unless we delegate this to custom parsing functions (but that start to be cumbersome).

Since both multiline and symbolic strings will end with `"%`, it will no longer be true that the start & end delimiters of a string must match. It will also no longer be the case that we can infer whether or not indentation must be stripped from the starting delimiter. This commit separates the opening and closing delimeter into two different types, and provides a more explicit API for working with them, which will be resilient to the addition of a new `StringStartDelimiter` in a later commit.

This commit implements the symbolic string syntax `s%"..."%`. These strings using the same parsing rules as multiline strings, but are then desugared to `Term::Array`s rather than `Term::StrChunks`.

matthew-healy · 2022-12-16T10:09:25Z

@yannham

The first question is: should we have symbolic strings be multiline by default, or offer a separate version for multiline strings?

I think starting off by only supporting multiline symbolic strings makes sense. Especially as it's possible to just write mutliline symbolic strings on a single line.

I can't currently think of a use case for a syntax like s"..." that isn't already supported by the multiline version, but maybe I'm missing something?

The second question, which is independent, is what to do with interpolated blocks inside mutliline strings.

In the current iteration I just ignore the indent parameter. My thinking on this was that symbolic strings will always go through post-processing before they ever end up as regular strings, so making assumptions about how the interpolated values should be indented feels like it might be unnecessary, as Nickel has absolutely no control over the ultimate string value (assuming one is ever created).

To look at your example from the comment above:

let bar = "alpha\nbeta" in
m%"
  foo
     %{bar}
  baz
"%

currently evaluates to

[ "
foo
  ", 
"alpha
beta",
"
baz"
]

This feels like how I'd expect this to behave (though obviously that's completely unscientific 😅) - it seems to make sense to give the exact value that was interpolated here. Though I suppose one place it could matter is if the symbolic string is intended to be evaluated into a language with meaningful whitespace... 🤔

Ultimately, I don't have particularly strong feelings & am happy with whatever you think makes sense. I do think what we have now is probably "good enough" for a first pass - what do you think about opening a separate issue for how to handle multiline interpolated values?

vkleen · 2022-12-16T10:23:43Z

I think @matthew-healy's example evaluation is the most reasonable. I wouldn't expect Nickel to process a string that's interpolated into a symbolic string at all. That's also in line with how normal string interpolation in indented multiline strings behaves.

yannham · 2022-12-16T11:31:50Z

That's also in line with how normal string interpolation in indented multiline strings behaves.

Precisely, I don't think this is the case:

nickel> let bar = "alpha\nbeta" in
m%"
  foo
     %{bar}
  baz
"%
"foo
   alpha
   beta
baz"

nickel> let bar = "alpha\nbeta" in
s%"
  foo
     %{bar}
  baz
"% |> array.foldl (++) ""
"foo
   alpha
beta
baz"

We carry explicitly the whole indentation of an interpolation (here e.g. 4 spaces for the %{bar} block) and re-apply that at runtime, once we've evalauted bar, to the beginning of each line. We can't do that, at least not trivially, for symbolic string, because the parsing function is the one reconstructing the string (if ever done) at the end. The best we can do is to pass this indentation. Maybe we don't care 🤷‍♂️ but I just wanted to raise the difference in behavior.

Ultimately, I don't have particularly strong feelings & am happy with whatever you think makes sense. I do think what we have now is probably "good enough" for a first pass - what do you think about opening a separate issue for how to handle multiline interpolated values?

Sure, this is the way to go.

yannham

🎉

vkleen · 2022-12-16T11:42:01Z

@yannham That's interesting! I tested with the following in test.ncl:

let s = "a\nb" in
  m%"foo
  in %{s}
  bar"%

and nickel -f test.ncl results in

"foo
  in a
b
  bar"

I think the indent behaviour only kicks in if the multiline string starts with whitespace?

matthew-healy · 2022-12-16T13:04:02Z

@vkleen I think it kicks in if the line the interpolation is on starts with whitespace, so e.g.

let s = "a\nb" in
m%"hello
    %{s}
test
"%

results in

hello
    a
    b
test

But because your example has the text in on the same line we don't count that as an indent.

yannham · 2022-12-16T13:04:13Z

I think the indent behaviour only kicks in if the multiline string starts with whitespace?

Yes 🙂 you can find more context in #197. Maybe this information should be somewhere else, in the documentation?

matthew-healy force-pushed the feature/symbolic-strings branch from b4d5879 to eefac48 Compare December 14, 2022 15:42

github-actions bot temporarily deployed to pull request December 14, 2022 15:45 Inactive

matthew-healy marked this pull request as ready for review December 14, 2022 15:57

matthew-healy requested review from yannham, vkleen, djacu and dpl0a December 14, 2022 15:57

vkleen approved these changes Dec 14, 2022

View reviewed changes

yannham reviewed Dec 15, 2022

View reviewed changes

Matthew Healy added 2 commits December 16, 2022 10:23

Desugar symbolic strings to arrays

da165db

This commit implements the symbolic string syntax `s%"..."%`. These strings using the same parsing rules as multiline strings, but are then desugared to `Term::Array`s rather than `Term::StrChunks`.

StringEndDelimiter::Multiline -> StringEndDelimiter::Special

e062346

matthew-healy force-pushed the feature/symbolic-strings branch from eefac48 to e062346 Compare December 16, 2022 10:16

github-actions bot temporarily deployed to pull request December 16, 2022 10:19 Inactive

yannham approved these changes Dec 16, 2022

View reviewed changes

matthew-healy merged commit c30ad1f into master Dec 16, 2022

matthew-healy deleted the feature/symbolic-strings branch December 16, 2022 13:07

matthew-healy mentioned this pull request Dec 16, 2022

Decide how interpolating a multiline string preceded by an indent into a symbolic string should behave #1006

Open

yannham mentioned this pull request Jan 11, 2023

The Syntax Bikeshedding Dojo, round 10: Symbolic strings #1039

Closed

chenrui333 mentioned this pull request May 17, 2023

nickel 1.0.0 Homebrew/homebrew-core#131191

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Symbolic strings #994

Symbolic strings #994

matthew-healy commented Dec 14, 2022

vkleen left a comment

yannham left a comment •

edited

Loading

yannham Dec 15, 2022

yannham Dec 15, 2022

matthew-healy commented Dec 16, 2022

vkleen commented Dec 16, 2022

yannham commented Dec 16, 2022

yannham left a comment

vkleen commented Dec 16, 2022

matthew-healy commented Dec 16, 2022

yannham commented Dec 16, 2022

Symbolic strings #994

Symbolic strings #994

Conversation

matthew-healy commented Dec 14, 2022

vkleen left a comment

Choose a reason for hiding this comment

yannham left a comment • edited Loading

Choose a reason for hiding this comment

yannham Dec 15, 2022

Choose a reason for hiding this comment

yannham Dec 15, 2022

Choose a reason for hiding this comment

matthew-healy commented Dec 16, 2022

vkleen commented Dec 16, 2022

yannham commented Dec 16, 2022

yannham left a comment

Choose a reason for hiding this comment

vkleen commented Dec 16, 2022

matthew-healy commented Dec 16, 2022

yannham commented Dec 16, 2022

yannham left a comment •

edited

Loading