_js: Operators represented as numbers instead of strings

[rob-myers]

I'm seeing e.g. Redirect.Op with value 57 i.e. a number, whereas the API asserts the type RedirOperator with a method String().

It seems that gopher uses the numeric value of the enum token in tokens.go.

It would be useful to have a lookup in the Javascript API e.g. from 57 to '<&'.

[mvdan]

These are enums, so their values are integers, not strings. I agree that there should be a way to stringify them - I'll check for the upcoming release.

[rob-myers]

But gopher could have represented them as e.g. js objects { key: string; value: number }, preserving more information.

This module is really great. I've written typings for all the nodes, and also a utility to convert the parser's output into a more typescript-friendly format.

[mvdan]

GopherJS simply translates the code and data structures, so I'm afraid it isn't designed to do anything fancier than that. The token type is simply an integer - Go doesn't have anything higher-level for enums.

If you have ideas for a better JS package, or for a package that's higher level than what I currently have, please feel free to contribute or publish extra packages. I'm by no means a JS expert :)

[mvdan]

Once the GopherJS issue above is fixed, you'll be able to do something like:

console.log(redirect.Op) // 57
console.log(redirect.Op.String()) // "<&"

[mvdan]

I forgot to actually link to the issue. It's https://github.com/myitcv/gopherjs/issues/26.

[rob-myers]

I intend to contribute, but will wait until I've done more testing.

1. I've written typescript definitions for your API including the syntax package.
2. I've written a 'cleaning' function for each 'parse node', e.g.

private ArithmCmd = (
    { Pos, End, Left, Right, Unsigned, X }: Sh.ArithmCmd
  ): ArithmCmd => ({
    ...this.base({ Pos, End }),
    type: 'ArithmCmd',
    Left: this.pos(Left),
    Right: this.pos(Right),
    Unsigned,
    X: this.ArithmExpr(X),
  });

Concerning (2):

• I want the parser output to be serialisable, so e.g. I can clone via JSON.{stringify,parse}.
• It mirrors your API, extending each node with type whose value is syntax.NodeType(node).
• The type of type is its value e.g. ArithmCmd.type: 'ArithmCmd'. This permits discriminated-unions, and other typescript inferences.

[mvdan]

I've written typescript definitions for your API

Would this be a package sitting on top of mvdan-sh, or would the typescript code be included as part of the same package? I know very little about how NPM packages work.

I want the parser output to be serialisable

Note that there's something kidna like this already - try shfmt -tojson <<<'echo foo'. Similarly to what you described, it only keeps the public fields of each struct, adds types, and generally makes the JSON a bit easier to deal with. I haven't written the backwards conversion, though, but it could certainly be done.

That code currently lives in cmd/shfmt, but we could make it more generic and expose it to JS too.

It mirrors your API, extending each node with type

Note that GopherJS also adds node.$type, but I added syntax.NodeType so that people can use the shorter ArithmCmd instead of the fully qualified type *mvdan.cc/sh/syntax.ArithmCmd. I used a global function instead of a method because that was simpler to do, but I could have recursively added a method to all the nodes too.

[rob-myers]

Would this be a package sitting on top of mvdan-sh, or would the typescript code be included as part of the same package? I know very little about how NPM packages work.

It goes in the root of your npm module mvdan-sh as a file named index.d.ts, see here.

Note that there's something kidna like this already - try shfmt -tojson <<<'echo foo'. Similarly to what you described, it only keeps the public fields of each struct, adds types, and generally makes the JSON a bit easier to deal with. I haven't written the backwards conversion, though, but it could certainly be done.

That code currently lives in cmd/shfmt, but we could make it more generic and expose it to JS too.

Thanks for the info. There are advantages to performing the conversion using typescript. For example, I use generic types e.g.

  type ArithmCmdGeneric<Base, Pos, Op> = Base & {
    Left: Pos;
    Right: Pos;
    Unsigned: boolean; // mksh's ((# expr))
    X: ArithmExprGeneric<Base, Pos, Op>;
  }

Both your ArithmCmd and the JSON-version are instances, with different type parameters. Then there is single source of truth (index.d.ts), so the conversion to JSON cannot get out-of-sync.

Note that GopherJS also adds node.$type, but I added syntax.NodeType so that people can use the shorter ArithmCmd instead of the fully qualified type *mvdan.cc/sh/syntax.ArithmCmd. I used a global function instead of a method because that was simpler to do, but I could have recursively added a method to all the nodes too.

Sure, I saw this. Since $type wasn't in the public API, I deduced the node type via property existence, e.g.

  private WordPart = (node: Sh.WordPart): WordPart => {
    if ('ValuePos' in node) {
      return this.Lit(node);
    } else if ('Value' in node) {
      return this.SglQuoted(node);
    } else if ('Parts' in node) {
      return this.DblQuoted(node);
    } else if ('Slice' in node) {
      return this.ParamExp(node);
    } else if ('TempFile' in node) {
      return this.CmdSubst(node);
    } else if ('X' in node) {
      return this.ArithmExp(node);
    } else if ('StmtList' in node) {
      return this.ProcSubst(node);
    }
    return this.ExtGlob(node);
  };

where:

type WordPart =
  | Lit
  | SglQuoted
  | DblQuoted
  | ParamExp
  | CmdSubst
  | ArithmExp
  | ProcSubst
  | ExtGlob

[rob-myers]

Actually, it would be helpful to have node.type where e.g. ArithmCmd.type() is 'ArithmCmd'. It would also help to have the JSON converter in the JS package.

Then my initial contribution would be to provide typings for the API, including the output of the JSON converter.

In fact, it would suffice for the output of the JSON converter to provide the type field.

[mvdan]

@rob-myers thanks for the suggestion - I've opened a separate umbrella issue for all these JS API enhancements, since this issue will be closed soon.