endaaman / tym

Lua-configurable terminal emulator
MIT License
185 stars 14 forks source link

Accept unquoted or remaining arguments as the shell command #101

Closed xlucn closed 1 year ago

xlucn commented 1 year ago

I tested these scenarios with nemo, with the following gsettings:

$ gsettings get org.cinnamon.desktop.default-applications.terminal exec
'tym'
$ gsettings get org.cinnamon.desktop.default-applications.terminal exec-arg
'--'
  1. "Open with" -> "Neovim" on text files does not open the text file, only a empty buffer.
  2. "Open in terminal" does not open tym in the current folder, but the folder the daemon started in. This only applies when tym is in daemon mode.

I am not sure if these problems can be worked around by changing the gsetting options. Otherwise, it must have something do with tym itself.

js-everts commented 1 year ago
  1. Shouldn't gsettings get org.cinnamon.desktop.default-applications.terminal exec-arg be set to -e. But even then I'm not sure this will work as expected because of how tym expects args for the -e flag. I have a hacky partial fix implemented here that you can test.

  2. Make sure you have the latest version and the latest daemon is running to open terminals in the current folder.

xlucn commented 1 year ago

@js-everts Thank you for the reply! Problems basically solved.

  1. I did notice the -e flag, and also the fact that the shell command need quoting. As tested, the flag change doesn't work. But your fix branch works!
  2. Indeed it's because I haven't restarted the daemon since v3.4.1 update, this version works well now.
js-everts commented 1 year ago

Just for completeness, I should mention my fix won't work if you try to open a file with spaces in it.

xlucn commented 1 year ago

I noticed. I looked into your fix's code, and saw the comment It would be much better if can store these as an array. That will indeed solve that issue.

endaaman commented 1 year ago

Sorry for late reply.

As you note, tym can not handle the options correctly. In this case, I think it is appropreate to use double dash option ( -- ).

For example in lxc,

$ lxc exec <container name> ls -l /usr

This won't work but

$ lxc exec <container name> -- ls -l /usr

This works. The options after -- is ignored by lxc command and they are processed together. As a UNIX command, this treatment traditionally exists. I guess the DE (cinnamon) also expects the treatment implicitly.

To implement this, I need some time to find the correct behavior as a normal terminal but I will do in due time. Please wait.

js-everts commented 1 year ago

@endaaman

For what its worth you can find my (half baked) take on this here. It basically follows your idea of using -- followed by commands:

$ tym -- less Dockerfile

While all this works (both in --isolated and --daemon). Lua can no longer set the command ('shell'). Since all the lua config fields and command line options come from the same table and since I have removed the '--shell' flag from that table, Lua no longer has knowledge about it. I'm not sure how to go about this (maybe special case 'commands'?).

Anyway, I think we would want something like this in lua word:

tym.set('command', {'prog', 'arg1', 'arg2'})

tym.setconfig({
    'command', {'prog', 'arg1', 'arg2'},
})

// or maybe even
tym.set_command({'prog', 'arg1', 'arg2'})
xlucn commented 1 year ago

I changed the issue title, since this is the main problem here. Feel free to improve it to better describe the issue.

Basically, tym could fit into more scenarios (including some gui file managers) if it accepts shell command options as some other programs, like st, alacritty or foot, do:

endaaman commented 1 year ago

The spec "Only when -e is provided, the following options are parsed differently" is a kind of the workarounds, I think. I feel that this kind of issues are equally problematic in other projects and that the general consensus is to use --.

xlucn commented 1 year ago

I think, which solution to choose really depends on the way of how other popular programs spawn terminal emulators. For example, tym has to work with most file managers on opening terminal in current directory and open text files with vim in a terminal, etc. So, to know whether they support -- maybe require some research.

xlucn commented 1 year ago

@endaaman Thanks for the latest release, I noticed tym now supports -- followed by the shell command. I found out I can also do tym nvim text.txt without --. Not sure if that's the intended behavior.


For anyone with similar issues, here are my research and solutions after this update:

There are different scenarios of "opening in terminal":

  1. Those with explicit configurations, e.g., i3, xfce(thunar), nemo. In those cases, you can configure with their configuration files, GUI settings app (xfce), or gsettings (nemo).

    These are actually less problematic, since they are configurable. The real problem is the 2nd case.

  2. Desktop entry with Terminal=true. Such as right click in nemo and choose "Open in Vim". This behavior is not controlled by nemo, but by how the desktop entry vim.desktop is executed. I finally found out, in this case, it's up to Gio to decide the terminal emulator, unfortunately in a hard-coded way, see the code here. So the hack is to "disguise" tym as gnome-terminal, which accept -- as well:

    ln -sf /bin/tym ~/.local/bin/gnome-terminal

    Alternatively, one can implement a xdg-terminal-exec command to spawn tym:

    cat > .local/bin/xdg-terminal-exec << EOF
    #!/bin/sh
    tym -- "\$@"
    EOF
    chmod +x .local/bin/xdg-terminal-exec