-
-
Notifications
You must be signed in to change notification settings - Fork 169
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
Spawn terminal job using a list (and other jobstart() opts) #219
Comments
@jedrzejboczar so it can't be instead of since that is a major breaking change for this plugin, it would have to be also taking a list the same way that jobstart can take a string or a list. In either case, I'd keep the toggleterm ID Regarding the
I'm not sure if I see where the duplication is happening, but I do want to fix this, can you clarify what you mean? Also, re. adding more arguments to |
I actually meant also, just used unfortunate words :) vim.fn.termopen('echo "hello"; #toggleterm#1')
-- hello
--
-- [Process exited 0] with: vim.fn.termopen({'echo', 'hello', '#termopen#1'})
-- hello #termopen#1
--
-- [Process exited 0] While And the above may be a reason of #184, because:
so it would execute e.g. So do you thing it would be ok to allow The reason why I am asking for this and why I created #218 is that I am writing a plugin that allows to spawn multiple tasks, selected via Telescope and discovered from JSON config files. These tasks are spawned in the background and you can then open their terminals via another Telescope picker. I already have a working demo but it requires #218. I won't have time to work on this for the next 2-3 days, but then I want to clean things up for a working version. |
@jedrzejboczar thanks for the detailed explanation, 🤔 so this whole method of appending the comment and using it in the title of the window I I guess plugins using toggleterm in this way are less interested in re-identifying across session, although this mechanism isn't just used per session but also across the plugin in the I think the mechanism might need to be addressed entirely rather than just allowing it to be ignored for this use case since a bunch of different things will probably fail to work |
There is probably no good solution to store this information via But during runtime, it should be easier to identify terminals by just keeping track of them in a Lua table I actually thought that using a List to spawn terminals would involve less work. I still think that it would be nice to have, but I wouldn't say it's an important feature (and the cost of spawning one more shell process is minimal). Now, when I think about it, an option to pass |
A random note: it just came to me that the solution would be to use a custom URI scheme for toggleterm terminals. So toggleterm buffers could use |
@jedrzejboczar the skin used by the neovim terminal is actually not under my control. It's chosen on purpose and is used by neovim to restore terminals when you reload a session so I can't actually change its structure which is why I currently just append a comment to the name of the terminal which includes the toggle term ID |
@akinsho Sure, I know that the current |
I think terminals have to follow neovim's native format, changing their names seems like it would cause a bunch of bugs or issues I'd definitely rather stay away from messing with something that has significance internally and could lead to unpredictable weirdness |
Would it be possible to use a list as terminal command instead of using a string? The jobstart() api it is possible to supply a list, which is often convenient when calling a process, because all the argument quoting will be done by
jobstart()
.I looked at the code of task:__spawn() and it seems that every command gets a comment with terminal id appended, e.g.
;#toggleterm#1
. I was wondering what is the reason for doing this? Are these id/filetype used somewhere else? Or would it be safe to remove it? This way it would be possible to just pass shell directly.I guess that this is also the reason why #184 happens. In
local cmd = self.cmd or config.get("shell")
a new shell invocation is created, but latertermopen
also starts a shell process, so there are 2 shells.And a follow up question: would it be possible to pass other opts to jobstart() api. The interesting option is
env
which can be helpful for starting jobs that e.g. need to first load some environment like when using Python virtual environments. Then we could just passPYTHONPATH
viaenv
instead of doing things likecmd = "sh -c 'source venv/bin/activate && python script.py'"
.I could create a PR, but wanted to hear your opinion first.
The text was updated successfully, but these errors were encountered: