Skip to content
This repository has been archived by the owner on Feb 8, 2019. It is now read-only.

Multi and single line comments #1

Open
juliankrispel opened this issue Jan 25, 2016 · 15 comments
Open

Multi and single line comments #1

juliankrispel opened this issue Jan 25, 2016 · 15 comments
Assignees

Comments

@juliankrispel
Copy link
Contributor

The coffeescript compiler unfortunately completely ditches single line comments and currently there's no easy way to insert multiline comments into the ast (ast-types doesn't make that easy).

@flying-sheep
Copy link
Contributor

why? am i missing something?

@juliankrispel
Copy link
Contributor Author

@flying-sheep feel free to give it a try it'd be pretty awesome to get some help on it.

Comment nodes differ from any other nodes (this is not obvious from reading the source) in that comments need to have a location object to be usable by the recast printer and they need to be attached to a block (this can be the block of a program or a function body).

The problem with having to have a location object is that the transpiler needs to know where in relation to other nodes the comment should be positioned, currently the transpiler doesn't know or care about location.

@flying-sheep
Copy link
Contributor

ok, so what i understood (or thought to understand) from your comment:

  1. nodes are organized in blocks. a block is a sequence of expression nodes or so
  2. blocks have info about line numbers and so on where nodes reside
  3. comments are attached to blocks
  4. apart from comments, the structure of code is completely independent from that location information, so you ignoring comments you can convert to JS using only the node structure

is that all right?

@juliankrispel
Copy link
Contributor Author

  1. nodes are organized in blocks. a block is a sequence of expression nodes or so

Simplified but yes.

In answer to 2,3,4

Generally yes, here's some more explaining:

Blocks are generally nodes themselves. If you parse a bunch of javascript code with recast, recast attaches a location object to every node that tells you about line numbers etc. Every node inside that block will also get a location object.

However, we're not parsing javascript code here. We're building an AST manually (with the information we get from the coffeescript compiler).
Generally, the part of recast responsible for printing code doesn't need location objects except when it comes to comments and here's my theory why:

All block expressions live in block.body, but comments live in block.comments. This is very awkward, because comment nodes are stored in a different place, the printer now needs to know about where to place these comments in relation to other nodes, at least that's my theory as to why recast will not print comments that don't have a location object.

@flying-sheep
Copy link
Contributor

and how do the location objects look? line/column coordinates? or node indices? or what?

because if it’s the former, the AST also has to know about how exactly the code will be rendered (e.g. regarding whitespace), which really makes it all awkward. suddenly it’s all no longer semantics with attached presentation, but instead semantics tied to presentational information. (once you change semantic information, you also have to change presentational information)

is this the case?

@juliankrispel
Copy link
Contributor Author

line/column coordinates, which makes the whole thing rather difficult.

@juliankrispel
Copy link
Contributor Author

I wonder what the reason is to treat comment nodes differently than any other nodes

@flying-sheep
Copy link
Contributor

hmm, looking at recast’s printComments function, it works if we’re OK with simply adding a comment node without loc.

then this will be skipped but this will be executed

else we need those parts to work (flow-like type annotations)

Node {
    comment: Comment {
        loc: Location {
            lines: Lines {
                skipSpaces: (start: number, backward: boolean) => number,
                firstPos: () => number,
                slice: (start: number, end: number) => Lines,
                ...
            },
            start: number,
            end: number,
            ...
        },
        leading: boolean,
        trailing: boolean,
        type: "Block" | "Line",
        ...
    },
    ...
}

@juliankrispel
Copy link
Contributor Author

Hi @flying-sheep. I really appreciate the company ❤️

I don't get what you're saying here:

hmm, looking at recast’s printComments function, it works if

It works if what?

else we need those parts to work (flow-like type annotations)

It's entirely possible that I'm missing something glaringly obvious, but how are type annotations related to this issue?

@flying-sheep
Copy link
Contributor

eh, sorry, edited the second part, forgot to finish the first.

it works if we’re OK with simply adding a comment node without loc.

then this will be skipped but this will be executed

@juliankrispel
Copy link
Contributor Author

@flying-sheep how would that work? Where would the comment be positioned? I thought we just established that it wouldn't work and benjamin sorta confirmed it right?

@flying-sheep
Copy link
Contributor

based on your explanation. but look at the code: isn’t it so that if you simply don’t have .loc, it will just be inserted before/after that node?

/e: yes it will, but it’s attached to the function body block, as you said (so it can be inserted before/after it)

as said in benjamn/recast#159 (comment), there are methods that reattach comments. maybe we can use one to reattach comments from the body itself to the body nodes (e.g. statements in a function body)?

@flying-sheep
Copy link
Contributor

hmm, maybe like that:

for each comment:

  1. identify tightest enclosing node (i.e. whose location info completely wraps the comment’s loc info tightest)
  2. among the enclosing node’s children, identify node on the same line as the comment (if any)
  3. attach as leading or trailing to that node

@juliankrispel
Copy link
Contributor Author

Hmm

@juliankrispel
Copy link
Contributor Author

@flying-sheep that sounds like a reasonable way of doing it 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants