Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add a function to extract specific line from TextLines? #5

Open
jhrcek opened this issue Feb 28, 2024 · 7 comments
Open

Add a function to extract specific line from TextLines? #5

jhrcek opened this issue Feb 28, 2024 · 7 comments

Comments

@jhrcek
Copy link
Contributor

jhrcek commented Feb 28, 2024

I'm currently looking at implementing some functionality in haskell language server which uses this library.

I would like to implement something like
getTextAtRange :: LSP.Range -> Rope -> Maybe Text
and
getLine :: Word -> TextLines -> Maybe Text

With the current api I can do it like this

What do you think about adding few high-level helpers like this to the library?

@Bodigrim
Copy link
Owner

I'd prefer

getLine :: Word -> Rope -> Text
getRange :: Position -> Position -> Rope -> Text

and same for TextLines instead of Rope, but let's wait for #4 to land first.

@jhrcek
Copy link
Contributor Author

jhrcek commented Mar 1, 2024

Great, I'm not in a hurry to implement it.
Just a question about your proposed types:
Wouldn't it make more sense to wrap the return types in Maybe?
You know, to distinguish between cases where given Word / Position x Position are "out of bounds"?

getLine 0 "hello" == Just "hello"
getLine 1 "hello" == Nothing

@Bodigrim
Copy link
Owner

@jhrcek #4 has landed, so feel free to take a stab at this issue.

I'd prefer to keep new functions consistent with existing split* functions, which do not return Maybe.

@jhrcek
Copy link
Contributor Author

jhrcek commented Apr 30, 2024

Thanks, will look into it in the next few days.

@Bodigrim
Copy link
Owner

Is there still an interest to work on getTextAtRange? Or shall I get HEAD released as is?

@jhrcek
Copy link
Contributor Author

jhrcek commented Jul 17, 2024

I think it would be very useful.
There are bunch of places in hls which would benefit from this and I believe using ready-made helper from text-rope would improve correctness in many places, which are currently extracting text using char-based indexing.
But I won't have time in the near future to implement it, so I think you can release HEAD as is.

@Bodigrim
Copy link
Owner

Thanks, I released https://hackage.haskell.org/package/text-rope-0.3

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

No branches or pull requests

2 participants