DavidFeldhoff / al-codeactions

MIT License
17 stars 8 forks source link

[Question] findNewProcedureLocation "Always ask" #148

Closed fvet closed 1 year ago

fvet commented 1 year ago

Hey @DavidFeldhoff

Has something changed on the findNewProcedureLocation options?

In the changelog, I read

Add "Create .. with advanced options" code actions with that a few more options are available like adding publishers directly to that procedure or placing the procedure at a specific place. That's also the reason why the property "alCodeActions.findNewProcedureLocation": "Always ask" was removed.

However, I find the 'advanced options ...' method somewhat too advanced in most scenarios. Often I don't want to add publishers at all, and always want to choose the position (previous 'Always Ask' option)

In scenario below, I start via an event subscriber, create a procedure and extract some code to a procedure. All functions are added above the current one, where it would be more logic to add them after the current one. (see last part of the animation for the suggested result)

I think this used to work better in the previous release, where I could specify the location of the new function via the 'Always ask' option. Any thoughts / suggestions on how to improve this experience?

Extract

DavidFeldhoff commented 1 year ago

You're right, I removed it. Based on telemetry it wasn't used that much. I was one of the users, but for me it was more that I wanted to do it a few times, but not all the times, which was why I supposed that everyone used it like that. Furthermore there've been some issues because it's an interactive mode while the other two are not. Because internally I reused the creation of the OnBefore and OnAfter publisher for the creation of an "advanced procedure" including the publishers and then the interactive mode would have shown up 3 times (for the procedure, the OnBefore publisher and the OnAfter publisher). Of course that could be solved somehow as well on another way, but that have been the reasons to explain it a bit.

Basically I see now 2 options Option 1: if local procedures shouldn't be sorted by name but instead added at the end of the local procedures, you could change your setting like that. Then you would have to create the procedure from the event subscriber only once per advanced code action and place it below your event subscriber (I agree that you have to hit two times more "enter" as you have to say if you want to add publishers and/or want to place the procedure at a specific position, but maybe that's doable). The other extracted procedure would be then placed below your existing local procedure automatically because of the changed setting. Option 2: Im going to add the option again. But as I don't know where to add the Event Publishers in the example I explained above, I'd have to pick one of the setting options (either place the publishers sorted by name or by range). Don't know if that's right. Showing 3 time the interactive mode is for sure also not a possibility. And adding another setting like "sort by in interactive mode" for this special use case is in my eyes also not the best option.

So yes, I could implement it again, but then I've to solve the explained issues. Or you say that Option 1 is also a possibility for you?!

Let me know what you think.

Best regards and nice weekend David

DavidFeldhoff commented 1 year ago

Hi Frederic, just want to catch up on this one: Assuming I would introduce the 'Always ask' setting again. If you're then using the advanced code action option, where should the publishers be placed? I imagine you only want to place the procedure at a special place right? For that scenario I need a good solution to introduce it again.. Would be happy if you share your thoughts on this

DavidFeldhoff commented 1 year ago

@fvet would appreciate your input

fvet commented 1 year ago

Sorry for the delay, will look into this today 👍

fvet commented 1 year ago

I understand the issue when you would add the 'Always Ask' option back to the 'Find New Procedure Location' setting. In fact, for the advanced action, it actually makes no sense to change any logic, since you can choose all via the interactive mode. Introducing an additional value would mix up things.

It was mainly for the non-advanced option, that I wanted to always select the function location again. Ideally, this would require a new setting, instead of adding a value to the current 'Find New Procedure Location' setting.

Having done some tests, I think I will use the advanced action from now on and skip the OnBefore/OnAfter publishers and select 'Place Procedure at Specific Position'. This would serve my needs. (I don't bother the 2 extra clicks ;))