dbuenzli / down

An OCaml toplevel (REPL) upgrade
http://erratique.ch/software/down
ISC License
84 stars 3 forks source link

Relax the cursor position requirement in getting doc/types #20

Closed zli closed 5 years ago

zli commented 5 years ago

I found myself constantly have to move cursor backwards in order to check the type or doc of an identifier which I've just typed in (or tab completed), as my cursor will always be after that identifier in these cases. And after my inquiry is done, I'll again have to move the cursor back to where it was.

It feels ergonomic if we can additionally allow the cursor to be right after an identifier when a user requests its doc.

The patch relaxes a little more than the above use cases, in that if there are other things (other than spaces) e.g. brackets between the cursor and the last identifier, it will work the same. I feel this is a no harm addition and brings a bit more convenience probably.

Signed-off-by: Zheng Li dev@zheng.li

dbuenzli commented 5 years ago

Thank you, this is a very good usability suggestion.

Your patch is included as 1ae04430e22f2b0c2c72, I significantly tweaked it in c6835f7785ab after introducing two utility functions in 9fba04140afec which I think make the implementation easier to follow. I believe the result should be equivalent to what you did. Scream if that's not the case.

zli commented 5 years ago

Hi @dbuenzli,

No problem, thanks for making the change!

Just one conner case I noticed

# ␣List.map

where the cursor is on a whitespace (or any non-identifer character) at the beginning of the line, followed by an identifier. I suppose it shouldn't return the doc for List.map in the above case, but it does so. The issue seems to related to the definition of find_prev, which is a bit ambiguous if the result is 0 (i.e. it could mean sth was found at index 0, or nothing was found at all).

The corner case has nil impact on user experience (someone might even like it), so not really needs fixing, just thought it's better to let you know.

dbuenzli commented 5 years ago

Well spotted ! I fixed this in 83ba5d1d6ceb10200cada6 to avoid glitchyness.

I agree find_prev is a bit weird (and maybe should be named differently) for the reason you mention. It works well for certain cursor movements though. There is a kind of assymetry between the left part (cursor can't be -1) and the right part of the string (cursor can be String.length s) but maybe the current behaviour of find_{next,prev} is not the right way of solving these problems.