fix unicode completion in square brackets #25094
Conversation
| @@ -462,9 +462,9 @@ function dict_identifier_key(str,tag) | |||
| isa(obj, Array) && return (nothing, nothing, nothing) | |||
| end | |||
| begin_of_key = first(search(str, r"\S", nextind(str, end_of_identifier) + 1)) # 1 for [ | |||
| begin_of_key==0 && return (true, nothing, nothing) | |||
| begin_of_key==0 && return (nothing, nothing, nothing) | |||
There was a problem hiding this comment.
Do the return statement on line 496, do anything with this change? This is also ok, as it gives the other types of completion a chance.
There was a problem hiding this comment.
Good question, probably not. But I have several questions regarding some of the code here, so I went with the minimal change. For example, it seems that line 462 (special casing Array objects) is no longer necessary. Probably processing the key (i.e. line 464) should be done after the check for being a dictionary on line 467, etc.
|
Bump; it would be great if this works in 0.7 and it's a small and, I believe, non-breaking change |
|
The blocker here, I believe is review. Is anyone particularly familiar with this code? |
|
Is it possible to add a test case? |
|
I am not sure how to test REPL code/behaviour. Should I just test the return values of these functions (i.e. Also, I am pretty sure that the current patch is exactly, a mere patch, but not the best way to actually clean up this code. I implemented this to get the discussion going, but some input from anybody familiar with this code would still be very useful. |
|
I don't remember why I closed this, could this have been closed automatically because I somehow accidentally deleted this branch from my personal julia fork? |
|
Well, this probably needs an update, so I'll need to find some spare time. |
| @@ -462,9 +462,9 @@ function dict_identifier_key(str,tag) | |||
| isa(obj, Array) && return (nothing, nothing, nothing) | |||
There was a problem hiding this comment.
This should be AbstractArray, I think?
| partial_key = str[begin_of_key:end] | ||
| (isa(obj, AbstractDict) && length(obj) < 1e6) || return (true, nothing, nothing) | ||
| (isa(obj, AbstractDict) && length(obj) < 1e6) || return (nothing, nothing, nothing) |
There was a problem hiding this comment.
Seems like this test should go before line 464
|
I have an updated version of this PR with tests, and will open a new PR shortly. |
This fixes #24705, but I don't know anything of the REPL code so I am not sure this is the way to go.
To me, the name
dict_identifier_keyseems to indicate that it should only return non-trivial valuesidentifier, partial_key, locwhen the object in front of the square brackets is aDict. Currently, onlyArrayobjects were special cased to returnnothing, nothing, nothing, whereas all other objects returnedtrue,nothing,nothing. I don't see the point of thetruevalue, since this is then only used incompletionswhere it means that for any object which is not a dict (or an array), followed by[, effectively all completions are disabled.