FloatermNew --name=xxx leaves an unlisted buffer after closing that floaterm instance.

[voldikss]

• Please make sure you have looked through the F.A.Q. first.

• Please make sure you are using the latest version of the plugin.

• Please use English as far as possible.

Describe the bug

To Reproduce

• Steps to reproduce the behavior:

  1. :FloatermNew --name=test

  2. After opening floaterm, type :ls~ and there would be two terminal instances:

       3u hR  "floaterm://test"              line 0
       5u     "term:///mnt/repo//13635:/usr/bin/zsh" line 1

     But there should be just one --- the floaterm:// instance

  3. Close it either using <C-d> or :FloatermKill

  4. Type :ls!, the terminal instance term:///mnt/repo//13635:/usr/bin/zsh still is there...

It's caused by this line, https://github.com/voldikss/vim-floaterm/blob/9e8d68d457b67cf55ccfaa9ff14c891a79114db3/autoload/floaterm/terminal.vim#L40

However, adding 0file above that line doesn't solve the problem....

[voldikss]

Hi @vn-ki

Sorry to bother you here, but could you give some suggestions about this problem? The --name was added in the commit 6a820215a9953bd45db5e98a99d2d1b1242875ba. This problem has existed since then, but util this morning did I ran into it. I do not have any solutions for the moment, please help when you have free time.

Thanks in advance!

[adigitoleo]

This looks to me like it is definitely being caused by the line that you linked above:

from :h :file

:f[ile][!] {name}   Sets the current file name to {name}.  The optional !
            avoids truncating the message, as with |:file|.
            If the buffer did have a name, that name becomes the
            |alternate-file| name.  An unlisted buffer is created
            to hold the old name.

I guess we need to add :0file before that.

:h :0file

:0f[ile][!]     Remove the name of the current buffer.  The optional !
            avoids truncating the message, as with |:file|.

These help messages were from neovim, I guess it is the same for vim? Suggested solution: :execute '0file | file ' . termname Probably needs some testing.

[voldikss]

@adigitoleo Thanks for your attention to this issue. However, as I have described above, 0file didn't solve the problem.

[adigitoleo]

@voldikss I just read that haha, sorry

[adigitoleo]

@voldikss Looks like it is not recommended to change terminal buffer name by design:

https://github.com/neovim/neovim/pull/3181

There are some workarounds in that thread, I'll take a look. Also, it might be worth looking at how other plugins tackle this, e.g. fzf.

[adigitoleo]

OK so the fzf vim backend does not do this. They leave the name as term://[some_URI]/fzf which I think is the default after running termopen(). Is there any other part of the API that relies on floaterms having a URI beginning with floaterm:// ?

[adigitoleo]

I just quickly commented out that problematic line and it seems that :FloatermNew --name=test sets the name into some other metadata as well because :FloatermShow --name=test still works. There is no extra unlisted buffer, but the floaterm is called term://[URI]/[&shell] instead... If it doesn't break anything maybe the exec line can be removed for now, it doesn't really make sense to rename the terminals since they are still :terminal buffers, right?

EDIT: Nevermind, I tested some more and without the URI change it doesn't distinguish floaterms anymore...

[adigitoleo]

After some more testing, I think we need a change to how --name works. Even with workarounds, changing the terminal URI will probably lead to more problems.

I think that opts[name] could be put in jobopts[name] instead. It seems that jobstart supports custom keys in {opts}, the question remains if these keys can be used later to distinguish different terminal jobs/buffers. I am not familiar with neovim job control but I will have a go at implementing this.

@voldikss Is there a reason why opts and jobopts are separate dicts? Sorry, it's not the right place to discuss implementation details. I'll set up a PR after some more research.

[voldikss]

Sorry for the late response.

Is there any other part of the API that relies on floaterms having a URI beginning with floaterm://

Hmm I guess no.

:FloatermShow --name=test

Just use :FloatermShow test.

There is no extra unlisted buffer, but the floaterm is called term://[URI]/[&shell] instead...

No, use :ls! and there is indeed 2 different buffers.

If it doesn't break anything maybe the exec line can be removed for now, it doesn't really make sense to rename the terminals since they are still :terminal buffers, right?

Yes, in my view the only reason seems that my statusline can display something like floaterm://xxx :joy:

After some more testing, I think we need a change to how --name works. Even with workarounds, changing the terminal URI will probably lead to more problems.

Yes, the only solution for the moment seems to remove that exec line. Since name is stored in b:floaterm_opts['name'], the method to get buffer number from termname should be rewrite,

https://github.com/voldikss/vim-floaterm/blob/615568a454e3dfd7a2d1571dcd8a2407f4897303/autoload/floaterm/terminal.vim#L111-L113

It seems that jobstart supports custom keys in {opts}

Yes, but unfortunately vim8 doesn't allow extra key-values ;(

Is there a reason why opts and jobopts are separate dicts?

Hmmm, you are right about that. It seems better to handle all options in one variable...

[adigitoleo]

@voldikss OK today I have more time, maybe I can start to work on this. Thanks for responding to my chaotic comments from before :smile:.

Yes, in my view the only reason seems that my statusline can display something like floaterm://xxx

Also, let's hope nobody has set up autocommands for BufEnter floaterm://* (that would be a bad idea anyway, FileType floaterm is better)

Yes, but unfortunately vim8 doesn't allow extra key-values ;(

I just installed the latest vim and you are right. There is an interesting option in term_start:

"term_name"       name to use for the buffer name, instead of the command name.

Maybe that could be useful, but it is probably safer to implement get_bufnr using b:floaterm_opts as you suggest. We can get things like fancy statusline by writing an airline/lightline integration later.

Get well soon :)

[voldikss]

There is an interesting option in term_start

Thanks, I'll look into this later.

but it is probably safer to implement get_bufnr using b:floaterm_opts as you suggest.

In that case we have to traverse all floaterms to get that named floaterm. Though I dislike it, there seems no other better solutions. Using a dict only make a big fuss over a minor issue.

We can get things like fancy statusline by writing an airline/lightline integration later.

No need to do that actually. For one reason, lightline/airline already has good support for the normal terminal(as my demo pic shown above), for another reason, there are no statuslines for floating/popup window by design so we have nothing to do with the statusline. 😭