pnp / vscode-viva

With the SharePoint Framework Toolkit extension, you can create and manage your SharePoint Framework solutions on your tenant. All actions you need to perform during the development flow are at your fingertips.
https://marketplace.visualstudio.com/items?itemName=m365pnp.viva-connections-toolkit
MIT License
34 stars 14 forks source link

🐞 [Bug]: When no terminal has yet been opened, toolkit opens it and runs `ulp package-solution` #92

Closed martinlingstuyl closed 9 months ago

martinlingstuyl commented 10 months ago

📝 Describe the bug

If I haven't yet run a gulp task, clicking the 'bundle' or 'package solution' actions will open a new Terminal window. It will then run the gulp command, but something goes wrong and it misses the first 'g' of 'gulp':

image

I have this every time I close the 'Gulp tasks' terminal window and click the action. If I run a new action with the gulp tasks terminal already open, no issue occurs.

👣 Steps To Reproduce

Open an spfx solution, execute an action (bundle or package solution)

💻 Desktop (please complete the following information):

Adam-it commented 10 months ago

hi @martinlingstuyl thanks for using Viva Connection Toolkit and reporting a bug 👍

TBH I am having some trouble reproducing it 😔. It seems I follow the steps you describe but I get the 'g' in the 'gulp' key word May I kindly ask you to have a look at the recording and double check if I am doing something wrong?

https://github.com/pnp/vscode-viva/assets/58668583/fe1bdba6-c943-4bd3-bdc5-c57fd0fb934a

I see you are using OhMyPosh 🤩. I was wondering if may this produce some problems 🤔. I am not sure as I am also uing it but a different theme. Could you switch (temporary) you default terminal in VS Code to CMD (yep that old thing) and recheck if the problem reproduces 😉? Switching the default terminal in VSCode is very easy. In the terminal section you may use the arrow next to the + icon and pick 'Select Default Profile' option

image

After that please select CMD (assuming you are using Windows 🤔😉)

image

You may follow the same steps to get back to your previous profile after your done 👍

Also could you provide which version of Viva Toolkit you are currently using (major v1.1 or latest pre-release v1.1.3?) You may check that by going to the extension tab and clicking on Viva Connections Toolkit extension. Next to the title you will see which version you are currently using image

martinlingstuyl commented 9 months ago

Hi @Adam-it,

Yeah I finally thought: let's try it out. 😁 You've done so much good work on this! The first thing I did caused this error, so I already assumed you would not have seen this before.

I tried some things out:

But I think it may be a timing issue. I'm doing some more imports and stuff in my posh $profile. If I comment everything away except oh-my-posh, it also works fine.

Version: I'm using 1.1.0 (checkout the original issue, it's on there as well)

martinlingstuyl commented 9 months ago

For example: I'm importing PSReadline and some predictors, I'm also setting the CLI m365? alias and command completion.

Adam-it commented 9 months ago

Yeah I finally thought: let's try it out. 😁 You've done so much good work on this!

Thank you for the kind words 🙏

The first thing I did caused this error, so I already assumed you would not have seen this before.

Aaaaaj. Sorry to hear the first experience with the tool was already shitty 😟. I am just before V2 release so I tested everything to be triple sure. I try to stand by what is done but still I ma unable to test all cases. Like how it behaves on Mac, or the case you had is also quite special

I tried some things out:

  • Default cmd profile works fine.
  • My powershell profile also works fine if I disable oh my posh.

So it seems we have found the culprit.

Ok. Thank you for the double check. Could you share which theme you are using? Or if it's custom could you send it over. I will try to repro Thai locally and fix it. Oh my posh is very popular so I guess this may be reproduced by someone else in future. This feature is actually very easy as I inject the command in terminal. I was wondering if your problem may be fixed if I just add a space at the beginning of the command. The command will still work and only the space will be lost (at least in your case)

Version: I'm using 1.1.0 (checkout the original issue, it's on there as well)

A yes indeed it is there. Sorry for not noticing. I strongly encourage you to try out the latest pre-release 1.1.3.

martinlingstuyl commented 9 months ago

It seems I revised my answer after you started replying @Adam-it :-)

I think it may be a timing issue. I'm doing some more imports and stuff in my posh $profile. If I comment everything away except oh-my-posh, it also works fine.

Aaaaaj. Sorry to hear the first experience with the tool was already shitty 😟. I am just before V2 release so I tested everything to be triple sure. I try to stand by what is done but still I ma unable to test all cases. Like how it behaves on Mac, or the case you had is also quite special

Of course, never mind, I'm not offended by bugs. 😁 The toolkit looks great and has some nice actions to save me typing. That's enough for me already. And I see everyone and they're mother using it!

martinlingstuyl commented 9 months ago

For example: I'm importing PSReadline and some predictors, I'm also setting the CLI m365? alias and command completion.

👆 and this

Adam-it commented 9 months ago

yes indeed I didn't notice the other comment 😅.

For example: I'm importing PSReadline and some predictors, I'm also setting the CLI m365? alias and command completion.

👆 and this

hmmm.. I see. TBH I am not sure how to resolve this problem for now 😅. I checked the VS Code docs and it seems the Terminal.Create method already has a wait for boot up procedure build in and it seems currently it's default max wait time is 10s 🤔. Is it possible that your terminal actually takes longer to boot up?

Also I think what profiles and oh-my-posh does may be considered something that is done after booting up so VS Code may already consider terminal as started but as a result the profile is not loaded yet.

I will need more time to check and research how VS Code extension may be aware if the terminal is fully started before injecting the command. Other approach would be not to use default profile but always some specified terminal but then it will be different depending on OS.

As of now as I ma just before v2 release (I was planning to do it today or tomorrow) I will not be able to fix this on time. But I already plan a v2.1 which will be a more technical minor release so maybe I will try to research your case for that one.

I will keep you posted 👍

And I see everyone and they're mother using it!

I wonder what the fathers are using then 🤔😊? Probably good old Notepad++ 😉

martinlingstuyl commented 9 months ago

Sure, and don't worry, I consider this a low prio bug. :-)

I will need more time to check and research how VS Code extension may be aware if the terminal is fully started before injecting the command. Other approach would be not to use default profile but always some specified terminal but then it will be different depending on OS.

Yeah, annoying issue. Just adding a sleep before inserting the command would also seem so random. Who knows someone else might have an even bigger $profile.

Adam-it commented 9 months ago

Sure, and don't worry, I consider this a low prio bug. :-)

I will need more time to check and research how VS Code extension may be aware if the terminal is fully started before injecting the command. Other approach would be not to use default profile but always some specified terminal but then it will be different depending on OS.

Yeah, annoying issue. Just adding a sleep before inserting the command would also seem so random. Who knows someone else might have an even bigger $profile.

I wonder if just adding a white space at the begging of the command would solve this problem 😜. But it's rather touchy hack 😉. I will try to do some experimenting around it

martinlingstuyl commented 9 months ago

😁 such a hack would be perfectly fine as well. No one would probably notice, and even if they did it's highly doubtful they'd mind.

martinlingstuyl commented 9 months ago

Maybe you could insert a couple of comments first.

for example:

###### CLEANING PROJECT #######
gulp clean

However, you'd need to know what shell it is to make sure you use the correct escape character

Adam-it commented 9 months ago

@martinlingstuyl if I would prepare such a hack would you be so kind to test it over? I could just send you over a packed .vsix (vs code extensions pack) and 2 stepper how to install it manually 😜🙂

martinlingstuyl commented 9 months ago

sure!

Adam-it commented 9 months ago

sure!

Great. I'll try to prepare it over the night

martinlingstuyl commented 9 months ago

No hurries...

Adam-it commented 9 months ago

No hurries...

Let's reschedule than 😅. Sorry I didn't manage yesterday 🤦‍♂️

martinlingstuyl commented 9 months ago

Absolutely low prio...

Adam-it commented 9 months ago

@martinlingstuyl since we have a direct contact I hope you don't mind I sent you the vs code extension package and instructions over basecamp. Thanks for testing 👍

martinlingstuyl commented 9 months ago

Of course not! Will check it out today!

martinlingstuyl commented 9 months ago

Works like a glove @Adam-it !

Adam-it commented 9 months ago

Works like a glove @Adam-it !

Awesome. I will include this hacky fix in V2 Thanks @martinlingstuyl for your support 👍

Adam-it commented 9 months ago

PR merged. Usually we create a minor release with the fix but since we are under major release it will be just part of the upcoming major. Thank you @martinlingstuyl for your help 👍