First pull request for comments on extract to label

DavidRoys commented 3 years ago

Hi David. OK, so it's taken a really long time to get this to you. I still can't get my tests to work because no matter what I do I cannot read my config setting after setting it. I did manual testing with the setting both on and off and it appears to work exactly as I required, but you might have different opinions about it.

Here are some test results:

        Test1txt: Label 'No Local Var Section end.';
        Test2txt: Label 'No Local Var Section %1 end.', Comment = '%1=';
        Test3txt: Label 'No Local Var Section %1 and %2 end.', Comment = '%1=; %2=';
        Test4txt: Label 'No Local Var Section %1 and %2 and %3 and %4 and %5 and %6 and %6 and %7 and %8 and %9 and %10 end.', Comment = '%1=; %2=; %3=; %4=; %5=; %6=; %7=; %8=; %9=; %10=';
        Test5txt: Label 'No Local Var Section %1 and %1 and %3 and %4 and %5 and %6 and %6 and %7 and %8 and %9 and %10 and %11 end.', Comment = '%1=; %2=<not used>; %3=; %4=; %5=; %6=; %7=; %8=; %9=; %10=; %11=';
        Test6txt: Label 'No Local Var Section %1 and %1 and %5 and %4 and %2 and %2 and %7 and %1 and %2 end.', Comment = '%1=; %2=; %3=<not used>; %4=; %5=; %6=<not used>; %7=';
        Test7txt: Label 'No Local Var Section %2 and %1 end.', Comment = '%1=; %2=';

Notice that no mater what order you put the placeholders in the text, they get shown in the comments in ascending order. If you miss a placeholder out (i.e. you use %1 %3) then I include a %2 with a description. This should hopefully alert the developer that something went wrong with their placeholders.

I see you've been busy making changes too so I merged your latest master before sending this through. I add a new codeunit to the sample project to use for testing which I can send through if you want, but maybe my tests are no good anyway.

DavidFeldhoff commented 2 years ago

Hi Dave, first of all thanks for the pull request :) Currently I'm on vacation and was before that quite busy, so sorry I did not have a look at your work already. I will look at it after my vacation.

All the best David

DavidRoys commented 2 years ago

No worries at all. Enjoy your holiday. I feel bad submitting this when I still couldn’t get the tests working, but figured it was silly to sit on it when I get so little time to work on it and have no clue why the tests are failing. Better to let you take a look and see if you can salvage anything. 😀

DavidFeldhoff commented 2 years ago

Hi Dave, I just looked into it and there was one nasty bug (in my tests as well - I don't know why it wasn't showing up there as well) The problem was that the WorkspaceConfiguration.update method is an async one and we did not wait for it to be finished. So we executed the test while the configuration wasn't updated correctly. See this commit https://github.com/DavidFeldhoff/al-codeactions/pull/125/commits/5066cb1b8fbe760e7e49f1d70fe1a595c4887041 and you see that the fix was quite simple, but difficult to find. With that your tests are running smoothly :) Thanks for them! I'll merge the changes into my develop branch and publish them together with some other changes into master. I appreciate your time you spend on this one and hope you had some fun as well while learning to develop VSCode extensions :)

All the best from germany to you. Cheers, David

DavidFeldhoff / al-codeactions

First pull request for comments on extract to label #125