Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Using go-org to build a multi-purpose LSP #91

Open
M3kH opened this issue Nov 2, 2022 · 4 comments
Open

Using go-org to build a multi-purpose LSP #91

M3kH opened this issue Nov 2, 2022 · 4 comments

Comments

@M3kH
Copy link

M3kH commented Nov 2, 2022

Hi I would like to experiment with go-org to build a LSP for org.
I'm new in go and so my observations might be not correct.

My intention is locate all the #+begin_src and use the language tag and grab the content and proxy to an appropriate LSP, which I control for proper integration.

I manage to do this with orgajs because the AST contains the position (start, end) of the Node.

To achieve the same thing with go-org I identify some approaches, but since I'm not an expert I'm looking for suggestions:

  1. Convert Document.tokens into public Document.Tokens, this is not perfect because for parsing information like language I need to go to AST format, but seems the easiest.
  2. Get the position from the block with either Document.parseOne or Document.parseMany by making sure that all Node are converted into *Node so that I can use the Pointer reference in a Linked List as [*Node]NodePosition.
  3. Move from type Node interface into type Node struct to include more information.

Although I'm willing to do a PR if this is from common interest, I'm asking for help to learn and to understand what is the best "golang way" for such case.

Thank you in advance.

@niklasfasching
Copy link
Owner

Hey,

Option 2 sounds best to me. We could also embed a position{Start, End int} struct into each node instead of keeping the positions in a separate map (then again, just different tradeoffs; the map would make it easier to only keep positions for blocks and not inline nodes... just brainstorming here)

I don't think I should say anything about what's the true golang way; I've committed quite some atrocities against it in this repo :D, e.g. https://github.com/niklasfasching/go-org/commit/14900e9/

So whatever you pick, happy for a PR. I'll eventually look into implementing this myself otherwise, no idea when though.

@M3kH
Copy link
Author

M3kH commented Nov 11, 2022

Option 2 sounds best to me. We could also embed a position{Start, End int} struct into each node instead of keeping the positions in a separate map (then again, just different tradeoffs; the map would make it easier to only keep positions for blocks and not inline nodes... just brainstorming here)

So I went trough and try some, and consider one more option, here my learnings:

  • Option 2) Is doable, and indeed you get to selectively apply the position. Although convert from Value to Pointer seems affecting Performance and delivers a "less nice" API, again I'm not an expert. But at the end implementation didn't felt right.

  • Option 3) I try to adding fields to Node but I didn't foresee the complexity of inline nodes. So I drop the solution half way. I've embrace the simplicity that some Node are just string and came up with Option 4.

  • Option 4) I wrap Node into struct RangedNode which I add the Position next to it. I got this somehow working, but I got some question in prospect of a LSP.

LSP Considerations:

  • Org-mode is parsed mainly by line, and rightfully so go-org Scanner is setup with the default Read in line break.
    Although, LSP works a lot of time with Range query, so is useful to store the position also in character in the document.
    Can be valuable to go-org add Range as same LSP defines it?

    struct Range {
      Start { Line int, Char int }
      End { Line int, Char int }
    }
  • Would you rather than wrapping consider moving away from interface Node which leaves the ambiguity (and the simplicity) to node just be a string, to just be a more "complex" type? like:

    struct Node {
       Content string,
       Range Range,
    }

    Is this a Breaking change?

@niklasfasching
Copy link
Owner

Hey! Thx for the PR. I'll try to take a look this weekend :).

@niklasfasching
Copy link
Owner

niklasfasching commented Nov 26, 2022

Sorry for not getting back on this as planned. Looks good!

Are you blocked by this getting merged?

I'm still pondering how I feel about wrapping in RangedNode (or the suggested Node struct rather than interface). No strong feelings against it, just wondering whether it could be even nicer to instead cast to RangedNode to keep the interface unchanged, e.g.

type RangedNode interface {
        Node
	GetRange() Range
}

type Range struct{ Start, End Position }

func (r Range) GetRange() Range { return r }

type Comment struct{ 
    Range
    Content string 
}

func (d *Document) parseComment(i int, stop stopFn) (int, Node) {
	return 1, Comment{Range{...}, d.tokens[i].content}
}

_, node := parseComment(...)
node.(RangedNode).GetRange()

Maybe I'm just being nitpicky here. Any thoughts?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants