Skip to content

add additional exports to public API#2910

Open
rchl wants to merge 8 commits into
mainfrom
feat/public-api
Open

add additional exports to public API#2910
rchl wants to merge 8 commits into
mainfrom
feat/public-api

Conversation

@rchl

@rchl rchl commented May 17, 2026

Copy link
Copy Markdown
Member

@jwortmann

jwortmann commented May 17, 2026

Copy link
Copy Markdown
Member

There are probably various other utility functions from views.py that could be useful to be exported in the plugin API. Whenever you want to implement the handling of a custom request or notification, there is a high chance that you need to create some LSP type from a position or a selection in the file. For example some more functions that I used in LSP-Tinymist and LSP-julia are position, region_to_range, text_document_identifier, text_document_position_params.

https://github.com/sublimelsp/LSP-Tinymist/blob/5173ee56e777e3587f4823ef82107905e7445cd5/plugin.py#L22-L25
https://github.com/sublimelsp/LSP-julia/blob/6575c619fe9604845ddbd0ef0b542e7943b662d5/plugin.py#L20-L21

If we want to export more of them in the public API, perhaps it would be useful to reconsider some of the names, because there is a bit of inconsistency. A few function names are in the form "X_to_Y" (point_to_offset, region_to_range), while others are only named after the resulting type (position instead of offset_to_position), and even a third variant of utility function names use a get_ prefix (get_uri_and_range_from_location instead of location_to_uri_and_range).

Also the Point name for that custom class is perhaps not the best name, because meanwhile ST has introduced a type with the same name (in the sublime_types module), representing just an offset (int alias): https://www.sublimetext.com/docs/api_reference.html#sublime.Point
Perhaps a better name for the custom class from this package would be TextPosition (because it corresponds to a LSP Position)?

@jwortmann

Copy link
Copy Markdown
Member

I think for backwards compatibility with packages that import Point directly from LSP.plugin.core.protocol, we could rename Point to TextPosition in the LSP code base, export TextPosition in the public API, and add this into the core/protocol.py file:

@deprecated('Use TextPosition instead')
class Point(TextPosition):
    pass

@rchl

rchl commented May 22, 2026

Copy link
Copy Markdown
Member Author

I think for backwards compatibility with packages that import Point directly from LSP.plugin.core.protocol, we could rename Point to TextPosition in the LSP code base, export TextPosition in the public API, and add this into the core/protocol.py file:

Did that but not using new name in the code as I don't want to make things more difficult for @rwols. Can be renamed internally at later time.

As for other utility functions and their inconsistent names, perhaps we could group those into some classes with just static functions. But then we'd have to come up with some logical grouping and naming which will probably take some time.

@jwortmann

jwortmann commented May 22, 2026

Copy link
Copy Markdown
Member

Did that but not using new name in the code as I don't want to make things more difficult for @rwols. Can be renamed internally at later time.

Ok. But I think you still need to update a few references to the Point class inside of the TextPosition class itself. I would also rename the parameter in from_lsp from point to position.

And as far as I can tell, the basic purpose of this class is to make the LSP Position comparable (by defining __lt__ method). I wonder could we get this automatically if we just derive from NamedTuple:

>>> from typing import TypedDict
>>> from typing import NamedTuple
>>> class Position(TypedDict):
...     line: int
...     character: int
...
>>> class TextPosition(NamedTuple):
...     row: int
...     col: int
...     @classmethod
...     def from_lsp(cls, position: Position) -> TextPosition:
...         return TextPosition(position['line'], position['character'])
...     def to_lsp(self) -> Position:
...         return {'line': self.row, 'character': self.col}
...
>>> p1 = TextPosition.from_lsp({'line': 3, 'character': 5})
>>> p2 = TextPosition.from_lsp({'line': 3, 'character': 10})
>>> p1 < p2
True
>>> p1.to_lsp()
{'line': 3, 'character': 5}
>>> p1.row
3
>>> p3 = TextPosition.from_lsp({'line': 3, 'character': 5})
>>> p1 == p3
True

@rchl

rchl commented May 22, 2026

Copy link
Copy Markdown
Member Author

That wouldn't have int coercion on init but I guess we don't really need that.

Also it would pass TextPosition(1, 2) == (1, 2) which we currently don't allow. Perhaps also OK?

Subclassing wouldn't work correctly for NamedTuple, though, I believe.

@rchl

rchl commented May 22, 2026

Copy link
Copy Markdown
Member Author

It can be a dataclass with a caveat that comparing Point and TextPosition won't work:

>>> from LSP.plugin.core.protocol import Point, TextPosition
>>> a = Point(1,1)
>>> b = TextPosition(1,1)
>>> a==b
False

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants