Fork me on GitHub
#calva
<
2019-06-13
>
pez09:06:48

I am trying to fix so that lein jack-in works on Windows with powershell. Hereโ€™s an experiment where I have simplified the quoting of the arguments. Works swell on my Mac. But I have no clue if it works on Windows with cmd.ex, nor with powershell. (My old Mac canโ€™t handle Windows 10 in Virtual Box.) Can I get some help giving it a try. @brandon.ringe, maybe? And everyone, really, because this is a potential major break of vital functionality.

slack103810:06:52

@pez with Powershell i get the exception java.lang.NumberFormatException: Invalid number: 0.6.0 and with cmd also i think the versions needs to be like thiis \"0.6.0\"

pez10:06:36

Thanks, @slack1038! I suspected that a bit. What happens in cmd.exe?

pez10:06:30

Interesting. Have some time to work at it together with me? Itโ€™ll be quicker than me building vsix packages blindly. ๐Ÿ˜ƒ

pez10:06:22

Cool! Iโ€™ll PM you.

pez10:06:37

I have updated the wiki page about evaluating code. Iโ€™d like feedback. โค๏ธ https://github.com/BetterThanTomorrow/calva/wiki/Code-Evaluation-Tips (ping @lspector )

borkdude11:06:35

gentle reminder, feel free to vote on a convention for tooling (something which calva may eventually also support, if it does things like clean-ns in emacs): https://github.com/borkdude/clj-kondo/issues/241#issuecomment-500805941

stefan.van.den.oord12:06:36

Thanks @borkdude. I think I may be trying to complicate the decision here, but I proposed another alternative (`^:require/implicitly_used`) that is hopefully more intention-revealing than imperative (โ€œkeepโ€). ๐Ÿ˜…

pez15:06:36

Good news! @slack1038 and I worked at this together and found a way to make jack-in work also for when powershell is the shell used by vscode. We tried for two solid hours to make nice changes that would make it work, but in the end we had to resort to a somewhat ugly workaround. Anyway, here is a build that works on both our setups. I am going to publish it soon, but am giving people here a chance to find problems before our users out there gets them. Thanks in advance for help testing this! โค๏ธ

brandon.ringe15:06:25

It works for me aw_yeah

brandon.ringe15:06:25

@pez @slack1038 Thanks for the work. Did you make it switch the shell to cmd before running the cmd-friendly version of the command? It did ask me to allow the workspace to modify my shell (to cmd when I had it set to powershell).

brandon.ringe15:06:32

It worked after I clicked allow

slack103815:06:59

Yes that is the ugly workaround for now

brandon.ringe15:06:17

Oh I see. Not bad though. The default PS shell is maintained

brandon.ringe15:06:36

I guess the other way would be to read what shell they have set and issue the proper command for that shell?

pez15:06:53

Yes, issuing the proper command for lein.bat in powershell proved too hard for us. We could construct a command line that worked when issued in the terminal, but not when it was issued from the jack-in task.

brandon.ringe15:06:06

ohh interesting

brandon.ringe15:06:09

Yeah I was wondering

brandon.ringe15:06:37

Several things at play together there. That's a tough one

brandon.ringe15:06:06

lein seems to do something with the formatting after it takes it in. PS wants a specific format. Issuing from jack-in probably does something to it as well

pez15:06:18

I guess that when the quoting requirements of the powershell clashes with those of whatever a .bat needs it gets REALLY complicated. We do get the quoting to work for cli-deps, but that is a .ps1 script.

brandon.ringe15:06:41

Is there a lein.ps1?

pez15:06:05

Donโ€™t know.

pez15:06:00

Do you have it on your machine?

pez15:06:54

The quoting for cli/deps is really complicated, by the way. Done in several steps and involves tripple backslashes and stuff. ๐Ÿ˜ƒ

brandon.ringe15:06:55

I could ๐Ÿ™‚

pez15:06:22

I think that if people donโ€™t have it on their machines then it isnโ€™t of much use for Calva.

brandon.ringe15:06:35

Oh, right, duh

pez15:06:56

Have you checked on your machine?

brandon.ringe15:06:16

Installed through Chocolatey and it puts lein.bat

brandon.ringe15:06:27

Interesting that they maintain it in /bin in the repo though

brandon.ringe15:06:00

They also advocate the .bat on the home page installation instructions

brandon.ringe15:06:31

This may be a very silly question, but can calva put lein.ps1 on someone's system during install? Like a dependency? Maybe even in a sandboxed way?

pez15:06:45

I guess it could be done. But I wonder if itโ€™s worth it. And first we should confirm that it would help, even. ๐Ÿ˜ƒ

brandon.ringe16:06:15

Probably not worth it right now with the other more urgent things. I'm looking into the go-loop printing issue just to see if I can make sense of what's happening and if I can try something new.

brandon.ringe16:06:42

I've just been using a "custom" calva where I return false for my workflow ๐Ÿ˜„

pez16:06:11

Thatโ€™s awesome. Makes me more happy than I had expected it to do!

pez16:06:16

Need help testing again. Added an escape out of jack-in for powershell users who do not allow the change of the setting. Ping @slack1038 and @brandon.ringe.

pez16:06:18

So to test ^ this ^ try with having both cmd.exe and powershell.exe set as shell default. And try with both allowing and not allowing Calva to change it when it needs to. Iโ€™m not sure how you reset things so that VS Code will ask again, but I hope you will figure that out. ๐Ÿ˜ƒ

brandon.ringe16:06:27

Somehow I cannot change my default shell -_-. The command won't work like it did before. I even checked the settings.json and it's set to something other than powershell when I change it, but powershell keeps being used when I open a new terminal

slack103816:06:13

you migth need to change the workspace settings

brandon.ringe16:06:30

That is it, thanks

pez16:06:20

Does that mean some users will still run in to troubles?

brandon.ringe16:06:35

possibly I'll check if it happens again

slack103816:06:17

the thing is we change the workspace settings from the code and i dont know how to do it on a global part that means if the user changes the settings globaly it wont change for the workspace itself

pez16:06:39

There is no global part. There is user and workspace. But maybe user has precedence over workspace?

brandon.ringe16:06:59

It appears that workspace has precedence over user

brandon.ringe16:06:01

I see what's happening

slack103816:06:02

then i mean user sorry

pez16:06:51

Global is probably a good name for it. ๐Ÿ˜€ Just got to use the vscode terminology here.

brandon.ringe16:06:03

So when I have powershell set as default shell in user settings (what the built-in command/task sets, and it is powershell by default on new install of vscode), then I run the jack-in command, calva is setting the workspace shell to cmd, running the jack-in command, and setting it back to powershell, but in the workspace

brandon.ringe16:06:27

Then when you use the built-in change default shell task, it changes the user settings, but the workspace overrides

brandon.ringe16:06:35

And is still set to powershell from Calva

pez16:06:07

So it is the restore that goes a bit wrong?

brandon.ringe16:06:14

It needs to change the user setting for default shell, not the workspace, if that is possible

brandon.ringe16:06:41

OR, it should remove the setting from workspace (at least if it was not there before)

brandon.ringe16:06:10

But if the workspace had another shell set to begin with, then changing the user one would not work for calva probably

pez16:06:00

I have pushed the code to master. I must do other things right now, but if someone else can look at it, the code is there.

slack103816:06:53

just as reference the link and yes this is the problem when the user changed its settings for the workspace we cant just removed it and i dont think there is a api to get the user setting and the workspace setting

pez16:06:24

I also don't think that api exists. Have had use for it before, but then it wasn't there.

brandon.ringe16:06:15

Well at least what we have now will work for people who just installed vscode and have PS set as default

brandon.ringe16:06:40

They just will have to go change workspace settings when they try to change their default shell after using the jack-in command

brandon.ringe16:06:46

No big deal though

brandon.ringe16:06:00

Better than it not working for a new install

slack103816:06:50

oh i found a way to find out what where the terminal is set with the method inspect

pez16:06:29

That gives me hope due that other feature I tried to add. ๐Ÿ˜€

slack103816:06:36

๐Ÿ˜… i cant start the compilation of calva anymore

slack103816:06:24

ah now it works xD yeah i have a whacky setup xD

slack103817:06:11

i just did a pull request with my improment ๐Ÿ™‚

pez17:06:09

Can you build a package and upload here? I'm afc.

slack103817:06:46

i have to go now to sport later tonight i can do it ๐Ÿ™‚

pez17:06:54

So that is. vsce package

pez17:06:08

Ok. Maybe I'll beat you to it. ๐Ÿ˜€

pez17:06:43

@brandon.ringe can checkout your PR and build his own for testing. ๐Ÿ˜€

brandon.ringe17:06:02

Have never done so but maybe I can later

brandon.ringe17:06:16

Trying to get some other work done ๐Ÿ˜…

pez17:06:02

Other work? ๐Ÿ˜

brandon.ringe17:06:06

Not calva-related, unfortunately, for my living income lol

pez17:06:30

Yeah, Calva dev is for poor people, Unfortunately. Haha

brandon.ringe17:06:48

Need to get Calva funded by Clojurists Together

brandon.ringe17:06:04

Though being an editor extension, I don't know if it could

brandon.ringe17:06:20

You could make like $3k per month for 3 months though

pez18:06:42

Fireplace got funding this time around. Just sayin'

pez18:06:12

Though my not-so-well-payed day job wouldn't allow me to go do this for three months, so it would take that someone else was ready to take the assignment.

pez18:06:49

Iโ€™m a bit tired right now, @slack1038, but it looks like you didnโ€™t base your changes on my latest changes. I know github says there are no conflicts, but I hesitate to pull the PR since it looks like it will revert stuffโ€ฆ

pez18:06:22

Quite strange actually. And could just mean I should close the lid on my laptop for today.

slack103819:06:12

Yeah i did something wrong But i had to make a merge because of it That is the second commit But i can also just revert my fork back and do the change again

pez20:06:09

Please do. I could probably fix it on my end, but it would be easier with a clean change.

gerred22:06:44

https://clojurians.slack.com/archives/CBE668G4R/p1560446670426900 disagree, I'd support this. calva oddly works perfectly for my workflow, really happy with it. especially combined with live share, remote development, etc.

gerred22:06:24

just discovered it and I love cursive and friends but pretty blown away.