DavidFeldhoff / al-codeactions

MIT License
17 stars 8 forks source link

Create Procedure: return value missing for page fields #76

Closed NKarolak closed 3 years ago

NKarolak commented 4 years ago

I've just added a field to a page. The value is returned by a (still missing) function/method CalcValue:

                    field("My Page Field Name"; CalcValue())
                    {                        
                    }

When I run "Create Procedure" on CalcValue, it has no return value:

    local procedure CalcValue()
    begin
        Error('Procedure CalcValue not implemented.');
    end;

I am aware that you cannot know which return data type to expect. Nevertheless, could a dummy data type, e.g. text, be added? Example:

    local procedure CalcValue() ReturnVariable : Text
    begin
                // Please update the ReturnVariable data type and name
        Error('Procedure CalcValue not implemented.');
    end;
DavidFeldhoff commented 4 years ago

Hi Natalie, I understand the use case.. But as this procedure definitively needs a return value, I think there are other solutions where the user does not have to have in mind that (s)he has to change the return type. Maybe I could ask directly which return type it should be like I already do by creating a handler function (see here). Or as I'm thinking about adding a "Snippet Mode" anyway to specify if the parameters are by "var" I could add the return value as placeholder as well.

What do you think about these alternative solutions?

NKarolak commented 4 years ago

I actually thought about creating a separate template, used only for page fields. Then, my suggested ocde comments would not affect anything else. Choosing the result type by user interaction sounds valid, too. I didn't quite understand the approach with snippets?

DavidFeldhoff commented 4 years ago

Let me try to explain: Let's say you're using the teventsubscriber snippet. Then you can jump with the Tab-Key through some placeholders like the object type, object name, event name etc. I was thinking to add to all parameters of the created procedure the "var" keyword and let the user jump from "var"- keyword to the next "var"- keyword, so that you can delete and tab through the declaration and decide on your own if the parameter should be by reference or by value. Is that clear so far? And maybe I could add the return type as another placeholder if one is needed, but could not be found by the extension itself..

What do you think of that approach from a UX point of view?

NKarolak commented 4 years ago

Sounds reasonable! 👍

NKarolak commented 3 years ago

Any plans to finish this issue ...? ;-)

DavidFeldhoff commented 3 years ago

Eehm... Yes of course. It's in the release I just published :D Sorry, I lost focus ;)

I have not realized it in the way described above. I just keep in mind that a return type is necessary and if the user decides to finally create the procedure, then a quick pick is shown to select the type which is returned. I hope that's fine for you this way?

procedure requires return type

NKarolak commented 3 years ago

I like it, thank you!