Implement backwards iteration for token.Cursor #429
Conversation
This adds methods to token.Cursor to implement backwards iteration with Prev and PrevSkippable. Methods Pop and PopSkippable have been renamed to Next and NextSkippable to match the backward counterparts. Seeking is implemented with the method Seek. This methods will be useful for reading attached and trailing comments.
emcfarlane
force-pushed
the
ed/seekCursor
branch
from
2c2e85c to
8a15978
Compare
This removes the Seek method in favor of a new constructor that sets the cursor at a given ID. The start and end bounds of the cursor are found when iterating through and encountering a close or open token.
emcfarlane
changed the title
| return Zero, report.Span{} | ||
| } | ||
|
|
||
| // Seek to the end. |
There was a problem hiding this comment.
This is now a range over the cursor to calculate the end. Is that okay? The callers might be able to be updated to infer the end from either knowing the cursor is over a stream or decl.
There was a problem hiding this comment.
I think all uses of this wind up exhausting the cursor.
I think that if it's seeking to the end, we should just make it a mutating operation, and rename it to something like SeekToEnd().
emcfarlane
force-pushed
the
ed/seekCursor
branch
from
3010da9 to
f801cdd
Compare
experimental/token/cursor.go
Outdated
| // This may return a skippable token. | ||
| // | ||
| // Returns the zero token if this cursor is at the beginning of the stream. | ||
| func (c *Cursor) BeforeSkippable() Token { |
There was a problem hiding this comment.
I think I would prefer if this was named something like PeekPrevSkippable or similar.
experimental/token/cursor.go
Outdated
| return c.stream[c.idx-1].In(c.Context()) | ||
| } | ||
| idx := c.idx - 1 | ||
| if c.isBackwards && c.idx >= 1 && c.idx <= len(c.Context().Stream().nats) { |
There was a problem hiding this comment.
Can you spill c.Context().Stream() into its own variable somewhere? It would help make this a little more readable.
Also, one way that you can simplify this is to use slicesx.Get(s.nats, idx).
There was a problem hiding this comment.
Switched to slicesx.Get.
experimental/token/cursor.go
Outdated
| if offset := impl.Offset(); offset < 0 { | ||
| idx += offset | ||
| } |
There was a problem hiding this comment.
Maybe something like
if impl.IsClose() {
idx += impl.Offset()
}There was a problem hiding this comment.
Use IsClose and IsOpen.
experimental/token/cursor.go
Outdated
| current := ID(c.idx).In(c.Context()) | ||
| impl := current.nat() |
There was a problem hiding this comment.
current is only used once; merge these lines.
| idx int | ||
| // The idx is the current token ID if this is a cursor over natural tokens, | ||
| // or the index into stream if this is a cursor over synthetic tokens. | ||
| idx int |
There was a problem hiding this comment.
I think the code below might be simplified if this is specified to be an index into stream.nats (i.e., a token ID minus one).
| mark := c.Mark() | ||
| tok := c.Next() | ||
| c.Rewind(mark) |
There was a problem hiding this comment.
I'm not a fan of this. I would prefer it if Peek did not perform a mutation, because that means it's not thread safe.
There was a problem hiding this comment.
Before Peek would mutate the cursor to advance over any skippable tokens, now it remembers the previous place to avoid mutating. But its not thread safe, easy for the caller to copy the cursor if its needed?
| return Zero, report.Span{} | ||
| } | ||
|
|
||
| // Seek to the end. |
There was a problem hiding this comment.
I think all uses of this wind up exhausting the cursor.
I think that if it's seeking to the end, we should just make it a mutating operation, and rename it to something like SeekToEnd().
This adds methods to
token.Cursorto implement backwards iteration withPrevandPrevSkippable. MethodsPopandPopSkippablehave been renamed toNextandNextSkippableto match the backward counterparts. The constructor has been altered toNewCursorAtwhich takes a single starting cursor. The bounds of the cursor iteration are now delimited by the open and closing token when iterating.These methods will be useful for reading attached and trailing comments.