[Bug] First hidden terminal is re-opened using {count}ToggleTerm command

[uloco]

If the first terminal I open in toggleterm is a hidden one with Terminal:new (floating), then :1ToggleTerm opens the hidden terminal instead a new one. Is there a way to prevent this?

My config:

local status, toggleterm = pcall(require, 'toggleterm')
if (not status) then return end

toggleterm.setup({
  open_mapping = '†', -- Alt-Gr + t
  insert_mappings = true,
  terminal_mappings = true,
  persist_size = false,
})

local Terminal = require('toggleterm.terminal').Terminal
local lazygit  = Terminal:new({ cmd = "source ~/.zshrc; lazygit", hidden = true, direction = 'float' })
vim.keymap.set({ "n", "t" }, "©", function() lazygit:toggle() end, { noremap = true, silent = true }) -- Alt-Gr + g

So basically I open lazygit with the mapping right after I open up vim and then put it in the background with the same command and then run :1ToggleTerm and it opens up again.

[akinsho]

@uloco I'm unable to reproduce this behaviour I use a similar setup but using 1ToggleTerm does not open my lazygit terminal. Can you confirm, what commit you are using of this plugin and also if you are setting hidden.

[uloco]

I am using tag = '*'.
Will try the latest version and report asap.

[uloco]

OK this still happens for me on the latest version too.
It only happens when there is no other terminal open and the first one is the lazygit one.
When I open a normal terminal first it does not happen anymore.

I have set the lazygit terminal to hidden as you can see above. neovims hidden setting is on by default also.

How I reproduce:

• Close everything and reopen neovim
• Press keymap for lazygit
• Press again to toggle
• Type :1ToggleTerm in cmdline
• lazygit opens again

[uloco]

It does not happen when I run the keymap with toggleterm (without a number)

[akinsho]

@uloco I can now reproduce it, seems specifically to happen when toggle is used to close the float as opposed to closing the window. Not sure what the difference is, but for my own part I close the float window with a q mapping rather than toggling it back closed since I've unmapped mode switching in toggleterm buffers.

I'm going to mark this as low priority and welcome any contributions as there are workarounds and I haven't got much time to look at it at the moment.

[uloco]

I'm happy to contribute, but I have no clue where to start :D

[akinsho]

@uloco so probably a good place to start is with

toggleterm.nvim/lua/toggleterm.lua

Lines 52 to 56 in 2a787c4

	local function smart_toggle(_, size, dir, direction)
	local terminals = terms.get_all()
	if not ui.find_open_windows() then
	-- Re-open the first terminal toggled
	terms.get_or_create_term(terms.get_toggled_id(), dir, direction):open(size, direction)

This part of the function to handle a command to open a terminal should check if a terminal with the number passed in exists and if so it should return it unless the terminal is hidden.

This happens here, the terminal should not be returned since it's hidden it should instead return nothing and create a new terminal but isn't for whatever reason.

toggleterm.nvim/lua/toggleterm/terminal.lua

Lines 508 to 513 in 2a787c4

	function M.get_or_create_term(num, dir, direction)
	local term = M.get(num)
	if term then return term, false end
	if dir and fn.isdirectory(fn.expand(dir)) == 0 then dir = nil end
	return Terminal:new({ id = num, dir = dir, direction = direction }), true
	end

[uloco]

Ok thanks. I'll have a look when i got time!

[uloco]

After investigating I found this:
In get_or_create_term, the num parameter is used as id to create a new terminal, although it is a hidden terminal. you should not create a new terminal with the id directly but rather check if it is hidden and create a new one as normal.
You have an idea how to do this quickly?

[akinsho]

@uloco I suggest looking at the terminals.lua file. I think it's not particularly complex. I don't really understand this statement.

the num parameter is used as id to create a new terminal, although it is a hidden terminal

so can't really suggest anything since I don't know what you mean. All terminal need an ID so there is some way to reference them so they don't get orphaned.

[uloco]

A new terminal is created because you don't check if the passed id belongs to a hidden terminal. It just creates a new terminal.

[akinsho]

@uloco please have a look at the following block of code you could maybe pass the include hidden argument when get is called and see if that works.

toggleterm.nvim/lua/toggleterm/terminal.lua

Lines 526 to 540 in 43b75f4

	function M.get_or_create_term(num, dir, direction)
	local term = M.get(num)
	if term then return term, false end
	if dir and fn.isdirectory(fn.expand(dir)) == 0 then dir = nil end
	return Terminal:new({ id = num, dir = dir, direction = direction }), true
	end
	---Get a single terminal by id, unless it is hidden
	---@param id number?
	---@param include_hidden boolean? whether or nor to filter out hidden
	---@return Terminal?
	function M.get(id, include_hidden)
	local term = terminals[id]
	return (term and (include_hidden == true or not term.hidden)) and term or nil
	end

I suggest seeing if always including hidden in get or create is the solution. You'll need to poke around locally and see if that produces the right outcome, this is about as much of an indication as I have short of just sorting it myself which I don't have the time to do.

[ulyssesdotcodes]

I was having a similar error, but it was happening on :ToggleTerm (without a number). I managed to resolve it locally in a different location. The toggle function doesn't take into account a terminal's existing size and direction, but it should so that the hidden floating terminal (lazygit in my case too) opens floating.

Changing to self.open(size or self.size, direction or self.direction) seems to fix. Could you confirm this is intended behavior and I'll make a PR? Happy to make the change in open too if that's better.

toggleterm.nvim/lua/toggleterm/terminal.lua

Lines 497 to 504 in 43b75f4

	function Terminal:toggle(size, direction)
	if self:is_open() then
	self:close()
	else
	self:open(size, direction)
	end
	return self
	end

toggleterm.nvim/lua/toggleterm/terminal.lua

Lines 468 to 492 in 43b75f4

	function Terminal:open(size, direction)
	self.dir = _get_dir(self.dir)
	ui.set_origin_window()
	if direction then self:change_direction(direction) end
	if not self.bufnr or not api.nvim_buf_is_valid(self.bufnr) then
	local ok, err = pcall(opener, size, self)
	if not ok and err then return utils.notify(err, "error") end
	self:__add()
	self:__spawn()
	setup_buffer_autocommands(self)
	setup_buffer_mappings(self.bufnr)
	if self.on_create then self:on_create() end
	else
	local ok, err = pcall(opener, size, self)
	if not ok and err then return utils.notify(err, "error") end
	ui.switch_buf(self.bufnr)
	if config.autochdir then
	local cwd = fn.getcwd()
	if self.dir ~= cwd then self:change_dir(cwd) end
	end
	end
	ui.hl_term(self)
	-- NOTE: it is important that this function is called at this point. i.e. the buffer has been correctly assigned
	if self.on_open then self:on_open() end
	end

[akinsho]

@ulyssesdotcodes I'm not sure I understand completely but hidden terminals should not be opened when using the ToggleTerm with count command they should only be opened via a reference to the terminal in lua. The fix you're suggesting doesn't seem to be about making sure the toggleterm command doesn't open hidden terminals at all?

[ulyssesdotcodes]

When I open a vertical terminal with :ToggleTerm and floating one with Terminal:new, sometimes they will both show up in the vertical terminal space instead of floating. I thought this was similar to this issue, but if not then I could open a new issue. My config for a regular terminal:

vim.api.nvim_set_keymap('n', "<leader>tt", ":ToggleTerm size=60 direction=vertical<cr>", {noremap = true, silent = true})

And my lazygit:

local lazygit = Terminal:new({ 
  cmd = "lazygit", 
  direction = 'float', 
  hidden = true,
  on_open = function(term)
    vim.cmd("startinsert!")
    vim.api.nvim_buf_set_keymap(term.bufnr, 't', '<esc>', '<esc>', {noremap = true, silent = true})
    vim.api.nvim_buf_set_keymap(term.bufnr, 't', '<S-esc>', '<cmd>close<CR>', {noremap = true, silent = true})
  end,
  on_close = function(term) 
      vim.cmd("startinsert!") 
  end
})

function _lazygit_toggle()
  lazygit:toggle()
end

vim.api.nvim_set_keymap('n', "<leader>tg", "<cmd>lua _lazygit_toggle()<cr>", {noremap = true, silent = true})

Not sure, but I think this is because the hidden terminal is being opened without referencing self.dir / self.size, which is the fix above.

[uloco]

I think I found the problem. It is in Terminal:new. If the first terminal I open is hidden it gets for example id 1. And when I use 1ToggleTerm, M.get will return nil (correct because hidden). But then you use Terminal:new which tries to reuse the existing id, although it is hidden and should not. The problem is that hidden terminals should not get the same kind of id's like normal terminals, because countToggleTerm will always also refer to hidden ones. and if you simply just use a new id, then toggle does not work anymore because the pressed counts and terminal ids do no longer match.

[uloco]

I worked around this issue by just giving my hidden terminals a very high id starting from 100 manually. Nobody will open 100 simultanous terminals so I guess it should be ok to do it like this. The question is, would it be ok to do it in your repo to? you could start ids at 1000 for example for hidden terminals.

[uloco]

another alternative would be: if there is a count specified only open the terminals that have the count set and is exactly the same. do not use ids as count but just as identifiers of whatever terminals are present.

[alexmozaidze]

Setting count to a really big value kinda solved it for me, but I did glance over the code to try solving the issue, and here's my proposal:

1. Remove count in favour of id
2. Remove hidden in favour of string ids
3. Make id of type number | string where number is for {number}ToggleTerm, and string is for ToggleTerm id=lazygit or Terminal:new { id = "lazygit" }

This should solve the issue and give more control to the user, but there be a lot of work to implement this; the entire codebase only assumes number ids.