Line objects should maybe have a char() function equivalent to InlineContext.char() #6

Closed
opened 2021-02-12 05:46:06 +01:00 by Monkatraz · 3 comments
Monkatraz commented 2021-02-12 05:46:06 +01:00 (Migrated from github.com)

Preface: This is a really minor "issue".

Currently, line objects implement similar methods to InlineContext objects but notably do not have a char() function, which feels disjointed / mildly disorientating (to me), considering the similarity of the two interfaces.

I would think this function would be document-relative, just like InlineContext.char().

To demonstrate my use-case:

function matches(points: number[], pos: number, cx: InlineContext | Line) {
  if (cx instanceof InlineContext)
    return points.every((ch, idx) => cx.char(pos + idx) === ch)
  else if (cx instanceof Line)
    return points.every((ch, idx) => cx.text.codePointAt(pos + idx) === ch)
  else return false
}

This function is a handy helper function to check if an array of specified code points can be found at a chosen position within the content of a Line or InlineContext object. Currently, this function behaves differently depending on whether you pass a Line object or if you pass a InlineContext object. With lines, the position is relative, and with inline, the position is document-relative.

I believe the reason why this isn't a thing is because Line is agnostic to its position within the document, so I understand if you don't want to implement this feature. I will say that, at least for me, the most common usage of Line is in the context of block parsers. Unless I am mistaken, in that context you will always have cx.lineStart available to you. Maybe a separate Line object type that includes document-relative position info and helper methods could be handy for block parsers?

And with this issue: I am done with my spam. Again, great work on this! It's awesome and I'm somehow having a lot of fun re-implementing my grammar for a third time.

Preface: This is a really minor "issue". Currently, line objects implement similar methods to `InlineContext` objects but notably do not have a `char()` function, which feels disjointed / mildly disorientating (to me), considering the similarity of the two interfaces. I would think this function would be document-relative, just like `InlineContext.char()`. To demonstrate my use-case: ```ts function matches(points: number[], pos: number, cx: InlineContext | Line) { if (cx instanceof InlineContext) return points.every((ch, idx) => cx.char(pos + idx) === ch) else if (cx instanceof Line) return points.every((ch, idx) => cx.text.codePointAt(pos + idx) === ch) else return false } ``` This function is a handy helper function to check if an array of specified code points can be found at a chosen position within the content of a `Line` or `InlineContext` object. Currently, this function behaves differently depending on whether you pass a `Line` object or if you pass a `InlineContext` object. With lines, the position is relative, and with inline, the position is document-relative. I believe the reason why this _isn't_ a thing is because `Line` is agnostic to its position within the document, so I understand if you don't want to implement this feature. I will say that, at least for me, the most common usage of `Line` is in the context of block parsers. Unless I am mistaken, in that context you will _always_ have `cx.lineStart` available to you. Maybe a separate `Line` object type that includes document-relative position info and helper methods could be handy for block parsers? And with this issue: I am done with my spam. Again, great work on this! It's awesome and I'm somehow having a lot of fun re-implementing my grammar for a third time.
marijnh commented 2021-02-12 10:17:54 +01:00 (Migrated from github.com)

I agree that the dealing with multiple coordinate systems (document, line, after-line.pos) is awkward. It was even worse in the inline parsers, which is why I added the convenience functions there. The thing is that line-level logic often needs to deal with actual line structure, where offset into the line is a lot more meaningful than offset into the document (when dealing with indentation and such). Since I didn't really run into a lot of trouble writing this logic, I decided against trying to abstract that away for the time being.

I agree that the dealing with multiple coordinate systems (document, line, after-`line.pos`) is awkward. It was even worse in the inline parsers, which is why I added the convenience functions there. The thing is that line-level logic often needs to deal with actual line structure, where offset into the line is a lot more meaningful than offset into the document (when dealing with indentation and such). Since I didn't really run into a lot of trouble writing this logic, I decided against trying to abstract that away for the time being.
danon commented 2022-01-31 13:47:06 +01:00 (Migrated from github.com)

Currently, this function behaves differently depending on whether you pass a Line object or if you pass a InlineContext object. With lines, the position is relative, and with inline, the position is document-relative.

Then clearly InlineContext and Line have different interfaces and can't be used interchangeably via polymorphic calls. I agree with marjnh, that it should be made similiar just to match abstractions; where-as the real use cases are different.

> Currently, this function behaves differently depending on whether you pass a Line object or if you pass a InlineContext object. With lines, the position is relative, and with inline, the position is document-relative. Then clearly `InlineContext` and `Line` have different interfaces and can't be used interchangeably via polymorphic calls. I agree with marjnh, that it should be made similiar just to match abstractions; where-as the real use cases are different.
danon commented 2024-04-23 15:35:50 +02:00 (Migrated from github.com)

Thanks for @marijnh !

Thanks for @marijnh !
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lezer/markdown#6
No description provided.