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

Indent expressions #25

Open
Jamesernator opened this issue Oct 12, 2020 · 12 comments
Open

Indent expressions #25

Jamesernator opened this issue Oct 12, 2020 · 12 comments

Comments

@Jamesernator
Copy link

Jamesernator commented Oct 12, 2020

Currently in the demo the behaviour of the following:

const func = String.dedent`
  function foo() {
  
  }
`;

const c = String.dedent`
  function bar() {
    ${ func }
    return foo();
  }
`;

Produces the following string:

function bar() {
  function foo() {
return 12;
}
  return foo();
}

While this matches the popular dedent library behaviour, it isn't particularly great as it makes it hard to compose separate indented chunks (not necessarily just of code).

I'd like to propose changing the behaviour so that the above snippet would produce the following:

function bar() {
  function foo() {
    return 12;
  }
  return foo();
}

This behaviour is currently implemented by the fairly popular npm library endent.


Note that this behaviour is not possible to add to the strings themselves because the indentation level is not known. For example this won't work:

const c = String.dedent`
  function bar() {
    ${ indented(func) }
    return foo();
  }
`;

Because the function indented would have no way of knowing the current indent.

An alternative strategy could be to expose a wrapper that allows embedded expressions to receive the current indentation level e.g.:

function indented(string) {
  // When String.dedent goes through each expression it checks if it's wrapped
  // by String.dedent.wrap, if it is then instead of converting the expression
  // into a string, it calls the wrapped function with the current indentation
  // so that it can apply the indentation
  return String.dedent.wrap(currentIndentation => {
    return string.split(/\n/g)
      .map(line => currentIndentation + line)
      .join('\n');
  });
}

const c = String.dedent`
  function bar() {
    ${ indented(func) }
    return foo();
  }
`;
@jridgewell
Copy link
Member

Going through this, I think it would only work for a non-tagged form. In tagged form, the dedented strings array is given to the tag without processing the arguments to the tag invocation (because it's up to the tag to do things with args).

In this specific case, couldn't the user just do a indent(string, 2)? They would need to keep that 2 up to date with the expected level out indentation, but avoids the sticky question of how the engine would handle this.

@Jamesernator
Copy link
Author

Going through this, I think it would only work for a non-tagged form.

Yes, although as you already mention tagged templates process the arguments anyway.

In this specific case, couldn't the user just do a indent(string, 2)? They would need to keep that 2 up to date with the expected level out indentation, but avoids the sticky question of how the engine would handle this.

Yes, although I don't agree that it would be a sticky question as to how the engine would handle it. endent already handles this fine and is a fairly tiny implementation.

I still feel like indenting embedded expressions is a more useful default for the majority of use cases for String.dedent (or triple-backtick, whichever lands). And given it isn't actually that complicated to implement, it seems worthwhile to include in the proposal.

@jridgewell
Copy link
Member

Sorry, I wasn't super clear in my comment. What I'm bringing up is the wrapped tagged template form (or a tagged dedent template if we have a syntax):

String.dedent(foo)`
  ${indent()}
`

// or
foo```
  ${indent()}
```

In this case, we don't interpolate the expression, we only dedent the template strings array. We then pass the dedented array and expressions to foo, which is responsible for interpolation. This is the sticky issue I mean.

@Jamesernator
Copy link
Author

Sorry, I wasn't super clear in my comment. What I'm bringing up is the wrapped tagged template form (or a tagged dedent template if we have a syntax):

String.dedent(foo)`
  ${indent()}
`

// or
foo```
  ${indent()}

Well like you say, this is ultimately up to foo, however if it were just to pass such values forward to String.raw (or hopefully String.cooked in future). This would require rewrapping with indent but is otherwise fairly simple:

function html(strings, ...expressions) {
    return String.raw(strings.raw, ...expressions.map(expr => {
        if (typeof expr === "string") {
            return escapeHTMLSomehow(expr);
        } else if (expr instanceof String.isIndent(expr)) { // 
            return String.indent(escapeHTMLSomehow(expr.value));
        } else if (expr instanceof Element) {
            return stringifyHTMLElementSomehow(expr);
        }
        throw new RangeError("invalid type of expression");
    });
}

String.dedent(html)`
  <html>
    <p>
      ${ String.indent(getTextSomehow()) }
    </p>
  </html>
`

This has a slight cost for those writing template tags, however for users of the tags they do not have to care how this internal processing occurs.

Although I think that yes in order to support custom tags, direct support in the existing String.raw would be neccessary. However I don't think it would be that hard to spec a branded object created by say String.indent (or whatever name) that gets contextually indented inside String.raw (or String.cooked).

@ljharb
Copy link
Member

ljharb commented Jan 30, 2022

It would be impossible, because then you’d have a function that had magic abilities attached to it that normally would require syntax. There’s all sorts of reasons that wouldn’t fly.

@Jamesernator
Copy link
Author

Jamesernator commented Jan 31, 2022

It would be impossible, because then you’d have a function that had magic abilities attached to it that normally would require syntax. There’s all sorts of reasons that wouldn’t fly.

There is no "magic abilities", libraries like endent already indent subexpressions today using standard template strings. And there's a few ways this could work but they all would just involve tagging expressions to be indented somehow, i.e. consider the following:

const s 
   = ```
       <html>
         <p>
           ${ someContent }
         </p>
       </html>
   ```

If say someContent were:

const someContent
  = String.dedent```
     <span>
       Hello <b>world!</b>
     </span>
  ```

Then as I'm proposing here it would be nice if this were indented by default into:

<html>
  <p>
    <span>
       Hello <b>world!</b>
     </span>
  </p>
</html>

Now perhaps this should be opt-in, in which case we'd just replace ${ someContent } with ${ String.indent(someContent) }.

But okay well how does this work with tags? Well whether the behaviour was default behaviour OR opt-in, one would just pass the indent objects straight through to the tag i.e.:

const s 
   = html```
       <html>
         <p>
           ${ String.indent(someContent) }
         </p>
       </html>
   ```

would pass the String.indent(someContent) through to the html tag. Now the html tag might choose to process it, or it might forward it along to String.raw (/String.cooked), in which a builtin indent behaviour will be used.

But again, there is no magic here, in fact this feature doesn't even need to part of this proposal, it would work with regular template strings anyway (i.e. how endent ALREADY WORKS today), BUT I think it makes more sense to be part of the proposal as in most DSLs people will probably wanting indenting by default if they are wanting indent formatted code to begin with. Like why would people want to strip indentation, but not correctly indent subexpressions? It seems very unlikely to me that people won't want the behaviour of indenting by default in the majority of cases where they are using String.dedent.

@ljharb
Copy link
Member

ljharb commented Jan 31, 2022

Maybe i misunderstood your comment about “branded object”.

It must not be possible for an interpolation containing any function call - which is just a normal expression context - to implicitly receive information about the source code that triggered it. That’s the magic I’m assuming you mean.

@Jamesernator
Copy link
Author

Jamesernator commented Jan 31, 2022

It must not be possible for an interpolation containing any function call - which is just a normal expression context - to implicitly receive information about the source code that triggered it. That’s the magic I’m assuming you mean.

The tags wouldn't know anything other than what is passed to them. In the explicit form with String.indent required then each of these calls just corresponds to the same call.

To have String.dedent (/triple backtick whatever lands) to have indenting by default, all it would essentially do is expressions.map(expr => String.indent(expr)) before passing it to the tag function. This however can't be distinguished from if someone just called the tag function with expressions.map(expr => String.indent(expr), in this regard there would be no magic. The fact that all expressions are wrapped could indicate a String.dedent, but this is no stronger than a function returning a Promise might be declared as async (ignoring the fact Function.prototype.toString does allow us to extract this info for real).

The branded object thing is just so that there is some way for tag code to actually understand the difference between String.indent(expr) and expr. This could be a class and using instanceof, my choice of String.indent simply branding was just because String.indent/String.isIndented feels simpler and more obvious to read than having ${ new Indented(expr) }.

The indent-by-default behaviour could be built easily on top of not-by-default, the main thing that is hard to do in userland though is that there is no common signal to template tags that something should be indented. Without this every single tag needs to reimplement essentially the same algorithm in order to get indenting working again, even though in most cases that isn't their primary goal (i.e. html would ideally leave formatting choices up to the user, but handle escaping and stuff well).

@Jamesernator
Copy link
Author

Jamesernator commented Jan 31, 2022

I also realized that a general indent(expr, indentDepth) doesn't work because in general expr might not be a string. For example an html expression might encode strings as text-nodes but might accept elements which would be encoded unescaped, but you would still want indent(someElement, 2), however if indent converts to a string rather than the html processor then it can't distinguish that this an element and shouldn't be escaped.

And sure every single library could provide it's own indent function, however given the algorithm for indenting is the same in pretty much all cases and is slightly tricky to implement, having this utility builtin would allow template tags to have less concern about having to re-implement this behaviour.

@Jamesernator
Copy link
Author

Jamesernator commented Jan 31, 2022

Just to simplify the above examples, all String.indent really does is wrap an object in an object that suggests to template tags that it should be indented. The builtin String.raw (/String.cooked) would just be extended to accept such objects and apply the indenting as requested. Hence tags that forward to String.raw (/String.cooked) could use the builtin algorithm.

Really all the String.indent object is just a box with a value + some styling hints about how this string could be formatted. It could plausibly be extended into String.format so that there is a common exchange format, but the reason I am not suggesting something much larger is that indentation seems to be the only formatting that String.raw could produce algorithimically that isn't tied to some specific DSL (i.e. colors would be more generic formatting, but this isn't something String.raw could produce, in some environments colors might mean ANSI, in others it might mean html wrapping elements and so on).

Although if there was interest in such a larger proposal allowing sharing formatting info with as many tags as possible, it could just be the case String.raw either throws-on or ignores formatting options it doesn't recognize.

@jridgewell
Copy link
Member

jridgewell commented Feb 1, 2022

I can see this being helpful enough that we can discuss it in committee. I think the simplest API is a String.indent() that returns a wrapper akin to:

class Indentable {
  #indentable; // this is an internal slot for the spec
  constructor(str) {
    if (typeof str !== 'string') throw new TypeError();
    this.#indentable = str;
  }

  // This is so that non-spec things can stringify
  toString(indent = '') {
    return AddIdentationTo2ndPlusLines(this.#indentable, indent);
  }
}

We'd then modify String.raw and String.cooked to see if the expression is an Identable, and if so, backtrack the buffer to capture the whitespace immediately previous to this interpolation point such that immediately before that whitespace is a newline char. We feed that whitespace slice into AddIdentationTo2ndPlusLines.

@Jamesernator
Copy link
Author

We'd then modify String.raw and String.cooked to see if the expression is an Identable, and if so, backtrack the buffer to capture the whitespace immediately previous to this interpolation point such that immediately before that whitespace is a newline char. We feed that whitespace slice into AddIdentationTo2ndPlusLines.

Yep this is pretty much endent and other libraries work today.

I think the simplest API is a String.indent() that returns a wrapper akin to:

Note that still generally makes sense to pass non-string objects to indentable, i.e. in HTML you might have ${ String.indent(htmlElement) } where htmlElement is a real HTMLElement object. The html tag would see Indentable(htmlElement) unwrap the htmlElement stringify it based on its own algorithm, then pass it back into a new Indentable before passing it forward to String.raw.

Although it might be the case that most libraries that generate DSLs as strings would want to pass to indent anyway in which case the user wouldn't even bother specifying.

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

3 participants