DavidFeldhoff / al-codeactions

MIT License
17 stars 8 forks source link

Extract to Procedure: not provided if leading space missing in selection #67

Closed NKarolak closed 4 years ago

NKarolak commented 4 years ago

For a quick test, please use this very stupid code:

if not (1 in [2, 3]) then begin 
end;

Imagine, you would like to extract the "complicated" expression into a procedure.

If you mark everything from ( to ), then the code action won't be not provided: image

It will be provided only if you mark the leading space: image

Bug or feature?

DavidFeldhoff commented 4 years ago

Good question. I think it's not in scope. If I remember correctly I only create the extract procedure code action if the whole if statement is selected (so that a boolean is the return type) and because you also select the space you hit the not expression which expands the range to not (...), so that you selected the whole if expression in that case (you can see that the not will be extracted also). I think that it's a little bit confusing in this case, that it does expand the range. I think I have to improve that one as well, so that it only expands the range if at minimum one character is selected.. But back to your request: Actually I don't know if that limitation is needed anymore. I'll put it on my list to think about, but this week I won't be able to make it.

NKarolak commented 4 years ago

Just to be sure I am not misunderstood: I really inteded to put the (1 in [2, 3]) part only into a new procedure. It returns a Boolean, too.

DavidFeldhoff commented 4 years ago

Yes, thanks for clarifying :) I understand what you're trying to achieve and I know that I can make that possible. I just have to find some time for that :)

DavidFeldhoff commented 4 years ago

Hi Natalie, in the next version I support to extract also partial expressions. The extraction of this "InList"-Expression would be a perfect example of such a partial expression. Before this I only supported the extraction of whole statements like a whole procedure call.

The if-statement was an exception as I allowed there to extract the whole (but only the whole) condition of the if. This was why the InList Expression couldn't be extracted - while the whole expression (not (1 in[2,3]) was possible.

NKarolak commented 4 years ago

Thanks David. Currently I cannot test it, but I'll close the case anyway.

DavidFeldhoff commented 4 years ago

Thanks for your patience as it took some time now