feat: add logic and tests for position at offset

[meymchen]

Description (e.g. "Related to ...", etc.)

A position_at_offset method is added to TextDocument, inspired by a nice work here.
Related tests are also implemented.

Code review checklist (for code reviewer to complete)

• Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
• Title summarizes what is changing
• Commit messages are meaningful (see this for details)
• Tests have been included and/or updated, as appropriate
• Docstrings have been included and/or updated, as appropriate
• Standalone docs have been updated accordingly

Automated linters

You can run the lints that are run on CI locally with:

uv run --all-extras poe lint

[tombh]

Great! Thank you ❤️.

Could you format with Black so that we can see just the changes you've made when reviewing the diff?

[tombh]

Great, thanks.

What do you think about putting most of this code in position_codec.py? Particularly I wonder if self._line_offsets: Optional[List[int]] = None is better suited there, or at least in its own class?

I must admit I don't know of a usecase for this function, which could just be my own inexperience. But what do you think about a sentence describing a simple usecase in the function's doc comment?

[nthykier]

I think this is missing cache invalidation.

My reading of the pygls code suggests pygls updates the TextDocument.source in-place if the client sends a delta for textDocument/didChange notifications but this cache would persist causing the line offsets to be out of sync with TextDocument.source.

A version check will suffice for documents open in the client. For server opened document, I am not aware of a good way to determine if the cache can be reused (since TextDocument.source will silently open and re-read the file every time).

[meymchen]

You are absolutely right. I rewrite the text document in my owned project and the dependencies is missing here. I'll update this in the next few days.

[nthykier]

Great, thanks.

What do you think about putting most of this code in position_codec.py? Particularly I wonder if self._line_offsets: Optional[List[int]] = None is better suited there, or at least in its own class?

Currently the PositionCodec is stateless, which has its own merits - namely, it can share it between all documents without any consideration and pygls seems to be doing this (per Workspace._create_text_document)

If we moved this logic to PositionCodec, then we would have to adapt the code to ensure PositionCodec is unique per document and get reset every time the document is updated (see my remark on the PR about cache invalidation). If we do this, I think we should bite the bullet and remove the lines argument from all the PositionCodec methods, since they would no longer be needed (I think this is a plus for moving away from stateless).

The downside is that pygls opens itself to a lot of cache invalidation/synchronization issues and would probably need a lot more tests to ensure that we catch such bugs when the code is tweaked from here on out. It is also a much bigger change than what OP might feel they signed up for. Additionally, it can affect implementation code that performs cross file checks (I think my code-base has a handful of those). I think it would also make sense to consider whether PositionCodec is still the best name for it if it is no longer stateless.

Personally, I think I am net for making it stateless in general with my current world view, since the passing the lines part of the API has felt awkward to me. I am likely not ready to do it given my own lack of progress on the ideas I had. So its comes down to OP or you having interest and capacity for it assuming we agree on the direction.

[tombh]

Thanks as always @nthykier, very good points. Let's keep self._line_offsets: Optional[List[int]] = None where it is. We just need to make sure it's updated whenever the text document is.