search

coder / sshcode

Run VS Code on any server over SSH.
MIT License
5.74k stars 216 forks source link

GitBash and MinGW support #132

Closed Merith-TK closed 4 years ago

Merith-TK commented 5 years ago

TL;DR

Summary of Edits

Progress

Latest Build:

deansheather commented 5 years ago

Is this still being worked on? The hardcoded C:\mingw64 thing is still an issue. Would like to merge this soon.

Meanwhile, sshcode seems to work on WSL now other than a few small hiccups.

Merith-TK commented 5 years ago

It is being worked on, i am slowly working on a home detector, see https://github.com/Merith-TK/Personal-Projects/blob/master/Home.go

Also school started a few weeks ago and i have been basically swamped. and i cant even test it at school during lunch because they blocked ssh... for some stupid reason

MOST RECENT PUSH FIXED THE ISSUE

eargollo commented 4 years ago

Is there any blocker to merge this PR? I am really interested in having MinGW support.

deansheather commented 4 years ago

@eargollo I've asked Merith over email for some changes before this can be merge. The whole mount scanning thing is too complicated for the sshcode tree, so we're going to default to the default mingw install location and have an environment variable for changing it.

Merith-TK commented 4 years ago

Oh crap! Got caught up with things my bad! Will implement this later today.

Merith-TK commented 4 years ago

okay @deansheather I am... Slightly annoyed, basically, Eargollo and i have made this function, and it does the job, And we made this function specifically for this, checking a env and verifying a path wont fully work and can lead to more bugs.

and a little more clarification on The whole mount scanning thing is too complicated for the sshcode tree? As this was not fully discussed in the email, what was discussed in the email was that the code was hard to follow, which i could say the same about the majority of the code in sshcode.go

The code works, is reliable, and does not require much user intervention except maybe adding a debugging flag to see what the mountpoints are returning in case its not properly slicing

EDIT: Reworded beginning

eargollo commented 4 years ago

To add to what @Merith-TK said, this code is only triggered for the MinGW use case, having a very restricted blast radius once it does not interfere with all of the ways sshcode is being used today.

We are basically adding another platform support.

deansheather commented 4 years ago

I mentioned a month ago over email that the mount code is too complex and hard to understand. You suggested an env variable they would overwrite the default location, and I said that's a good idea.

I do want to get msys and windows platform support up to spec with Linux. We should opt for a simpler route where the msys install dir is hardcoded, with an env variable to override it.

On Fri, 13 Mar. 2020, 8:20 am Eduardo Argollo, notifications@github.com wrote:

To add to what @Merith-TK https://github.com/Merith-TK said, this code is only triggered for the MinGW use case, having a very restricted blast radius once it does not interfere with all of the ways sshcode is being used today.

We are basically adding another platform support.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cdr/sshcode/pull/132#issuecomment-598457815, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACVYSVFGW2FA75MW5CT76MDRHFN3DANCNFSM4IL2Y4CA .

Merith-TK commented 4 years ago

We agreed that the code was a bit difficult to follow, not that it was "too complex for sshcode" and i have come to realize that an environment variable would be a bit too buggy as it wouldn't differentiate between git4win, and msys2, as the two have different base install paths, and there would be no simple way of acquiring this path that wouldn't inconvenience the user

EDIT: Also another reason against it would be that there would be way to many different ways someone could write their path, they could write it like so C:\msys2 which would return an escaped m C:/msys2, /msys2, people who dont understand the instructions, /, or msys2

Also this mount method can open up being able to support windows cmd.exe in the future as all we would have to do is tweak the pathing a little and detect that we are in cmd,

deansheather commented 4 years ago

In that case we should just add a section to the README which says what env variable to use depending on git bash or msys.

Merith-TK commented 4 years ago

I still don't want to rewrite the entire path fixing function but I am working on it since the F***ing coronavirus has canceled my plans. https://cdn.merith.tk/workspace/merith/go-sshcode/sshcode.go 2020-04-07 20:22:10 INFO Resolved windows path 'C:/msys64/Workspace' to '/msys64/Workspace is where I am at at the moment, What would be helpful is a command like winpath in wsl. because that thing you feed it a unix path, it will give the windows equivilant

EDIT, i am running sshcode.exe developer@192.168.0.115 --skipsync /Workspace

EDIT2: I am ready to throw my computer out the window { cd / && pwd -W; } | sed 's|/|\\|g' returns C:\Program Files\Git on gitbash and C:\msys64\ on msys

Merith-TK commented 4 years ago

one second, need to undo a personal modification

Merith-TK commented 4 years ago

Don't know whats going on with the current Travis build for you guys, but the build for me is working... https://travis-ci.org/github/Merith-TK/sshcode

kylejbrk commented 4 years ago

Any updates on this? Looking forward if this can run via Gitbash/Windows.

Merith-TK commented 4 years ago

@kylejbrk

Any updates on this? Looking forward if this can run via Gitbash/Windows.

My fork is completely functional for gitbash, msys2, and mingw. waiting on it to be merged

You can clone my repo in its current state and build it yourself if you like

Merith-TK commented 4 years ago

Dont know what broke travis for you guys, but it passes on my end https://travis-ci.org/github/Merith-TK/sshcode

deansheather commented 4 years ago

Please re-request a review when you want this to be reviewed and merged. I don't read most of my github emails because I get too many, but I get a notification on slack if I'm requested for review.

deansheather commented 4 years ago

I can't merge this automatically because of glitch with travis. The build passed on travis but didn't get reported to github. I'll squash and merge this manually tomorrow.

Merith-TK commented 4 years ago

Alright, hope people find this useful!

deansheather commented 4 years ago

Squashed and merged via 50e859cd1084379374420cb5263d053eae3b0254