"Extract to procedure" doesn't work well with regions

[achim-t]

codeunit 50120 Debugger
{
    #region DoSomething
    procedure DoSomething()
    begin
        Message('did something');
    end;
    #endregion DoSomething
}

Extracting the message:

codeunit 50120 Debugger
{
    #region DoSomething
    procedure DoSomething()
    begin
        ShowMessage();
    end;

    local procedure ShowMessage()
    begin
        Message('did something');
    end;
    #endregion DoSomething
}

I would have expected the new procedure to be located after the region.

[DavidFeldhoff]

Hi Achim, I assume that this does not apply to all procedure extractions. In some scenarios you would maybe want to have that procedure inside your region, that's why I don't want to change the default behaviour. So let's think about a different solution. What you could do by now already is, that you set the setting "alCodeActions.findNewProcedureLocation": "Always ask" With that you get a question where you want to place the procedure exactly.

https://user-images.githubusercontent.com/53570297/191540646-3dff4c17-6fa5-4937-ba7e-c7689c3fbb72.mp4

I use it myself also often, but I thought as well that a setting might be too strict. Because sometimes I want to have it placed just to the other local procedures (without further asking me) and sometimes I want that it is placed at a specific position. So maybe a second code action would then be great. What do you think?

[achim-t]

Hi David.

The region could be viewed as part of the procedure (or trigger etc.) if they have the same name. That way the anchor can be found (almost) like before, and the function can be placed inside or outside of regions. Assuming we want to extract the message call:

{
    //#region DoSomething
    procedure DoSomething()
    begin
        Message('hello world');
    end;
    //#endregion DoSomething
    //<- PLACE HERE
}

{
    //#region local procedures
    procedure DoSomething()
    begin
        Message('hello world');
    end;
    //<- PLACE HERE
    //#endregion local procedures
}

{
    //#region local procedures
    //#region DoSomething
    procedure DoSomething()
    begin
        Message('hello world');
    end;
    //#endregion DoSomething
    //<- PLACE HERE
    //#endregion local procedures
}

I don't like the alwaysAsk option and would prefer a second code action if the above can't be achieved.

[DavidFeldhoff]

Okay, that's something different. I think that's something we can all work with. I'm working on it.

[DavidFeldhoff]

Version is released. I hope it is how you want it to have. Otherwise feel free to reopen this issue :)

[achim-t]

Thanks, this works great (as soon as I use #region instead of //#region).