Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

open config file in local EDITOR, if user is connected via SSH #30

Merged
merged 1 commit into from
Oct 10, 2017

Conversation

zealot128
Copy link
Contributor

I am always working remotely on a linux server machine.

Thus, opening via the node-open doesn't work, the "gtt config" will never return.

When I am using xdb-open path-to-config.js (what the node-open package does under the hood), I am getting these messags:

$ xdg-open src/gtt-config.js      
Error: no "view" mailcap rules found for type "application/javascript"
Opening "src/gtt-config.js" with Neovim  (application/javascript)
xterm: Xt error: Can't open display:
xterm: DISPLAY is not set
/usr/bin/xdg-open: 461: /usr/bin/xdg-open: links2: not found
/usr/bin/xdg-open: 461: /usr/bin/xdg-open: links: not found
/usr/bin/xdg-open: 461: /usr/bin/xdg-open: lynx: not found
(...then w3m starts to "download" the file...)

AFAIK the whole xdg stuff is for GUI apps/multi terminal environments. For a terminal application, i would expect it to open in the EDITOR environment variable, like git commit does.

The proposed change checks the environment for known SSH variables (1). If those and the EDITOR is found, that is launched instead.

@coveralls
Copy link

coveralls commented Oct 6, 2017

Coverage Status

Coverage remained the same at 78.344% when pulling b956d32 on zealot128-os:ssh-login-editor into 8c835d7 on kriskbx:master.

@kriskbx
Copy link
Owner

kriskbx commented Oct 6, 2017

Very good point. I was aware of that but postponed any work on this because I never used gtt remotely. Thank you so much for your contribution!

I'm thinking about going one step further and completely removing the SSH_CLIENT and SSH_TTY check as well as the open package, only using the EDITOR environment variable. If EDITOR isn't set, gtt could show a warning as well as the path to the config file. So the user can open it manually. What's your opinion on this?

@zealot128
Copy link
Contributor Author

Sounds good, also the node-open package seems to be unmaintained, one more point for dropping it.
I am not sure if some people are relying on the "open" functionality at the moment, so changing it which might break some peoples work.

Another suggestion would be to check for $VISUAL first, and then $EDITOR. That would prefer gui editors if such variable is set (https://unix.stackexchange.com/questions/4859/visual-vs-editor-whats-the-difference).
Also might need to catch if neither is set, maybe fallback to "open" or quit with error message. Windows for example seems to have no such variables by default.

@kriskbx
Copy link
Owner

kriskbx commented Oct 10, 2017

Good point. I'll keep the open module as fallback for Windows then. Really appreciate your contribution and thoughts: I would just have deprecated the visual editor completely with the upcoming release.

@kriskbx kriskbx merged commit b956d32 into kriskbx:master Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants