Parse Comments line by line

[stasm]

Spin-off from #160; the Comment part.

[Pike]

This one I like.

There's a bit of a naming/AST-ishness thing, though. The relationship between the CommentLine, the used-to-be abstract baseclass BaseComment and the "no BaseComment in the AST".

I'd love to see a comment somewhere. Maybe this is something to add to spec/valid.md? Like, an AST with a BaseComment would be illegal? But also, we make 'em illegal and then fix 'em up.

Some human readable "comment lines of the same comment type are aggregated", and "Comments preceding a Message or Term without blank line are removed from the resource, and added to that Message or Term". I really don't know where to best put it, in grammar.mjs->fluent.ebnf, in abstract.mjs, or in spec/valid.md.

Slightly derailing, I wonder if there's a better way to pass non-public Nodes from the CST to AST generation? I toyed with the indention part, and had a similar urge for an Indent node in ast.mjs, and felt dirty about that. Would using PatternElement have been fine?

[stasm]

I'd love to see a comment somewhere. Maybe this is something to add to spec/valid.md? Like, an AST with a BaseComment would be illegal? But also, we make 'em illegal and then fix 'em up.]

I don't want to create too many assumptions about the AST. Some languages will need to slightly diverge from it to be able to implement Fluent. Right now, ast.mjs is not normative; it's more of a guideline.

I think we could standardize the JSON-serialized version of the AST. This would be useful for transporting data between implementations. Abstract classes like BaseComment and PatternElement would not be part of it, however.

Some human readable "comment lines of the same comment type are aggregated", and "Comments preceding a Message or Term without blank line are removed from the resource, and added to that Message or Term". I really don't know where to best put it, in grammar.mjs->fluent.ebnf, in abstract.mjs, or in spec/valid.md.

I'll add /* */ comments in grammar.mjs. They will be exported to the EBNF, too.

Slightly derailing, I wonder if there's a better way to pass non-public Nodes from the CST to AST generation? I toyed with the indention part, and had a similar urge for an Indent node in ast.mjs, and felt dirty about that. Would using PatternElement have been fine?

Yeah, perhaps! I used BaseComment as a means to "tag" the output of the CommentLine parser. You could use PatternElement to do the same. Or, just pass the raw indent (not wrapped in any {type, value} object) into the code in abstract.mjs. The fact that it's not an AST node can also be used to discriminate it.

[stasm]

Yeah, perhaps! I used BaseComment as a means to "tag" the output of the CommentLine parser. You could use PatternElement to do the same. Or, just pass the raw indent (not wrapped in any {type, value} object) into the code in abstract.mjs. The fact that it's not an AST node can also be used to discriminate it.

One more thing. The way I used BaseComment in the first comment, I didn't create an instance of it at any time. It's literally just for the purpose of matching the right case of a switch. I could have used any other value which is valid in a case comparison, like a string.

I'm not familiar with your exact use-case, but if you feel the need to temporarily store the indent as a node in the AST and then do something with it during a construction of another node, then we should look at the code and figure out if PatternElement is the right tool for it :)

In the second commit of this PR I dropped BaseComment in favor of a regular Comment.

[Pike]

Thanks. r=me.