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

feat: add option to execute tvp node command on leaf toggle #440

Merged

Conversation

antosha417
Copy link
Contributor

This was proposed in this discussion. And for me this option would make tvp feature more pleasant to use.

@antosha417
Copy link
Contributor Author

I'm not sure why the tests are failing 😓

Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pr @antosha417, although I'm still not sure I want to do this. You mentioned in the discussion that this is common in other tree-like plugins. Could you give me some examples? Also if we do this, I don't think I'd want it as another setting. I try to avoid having configuration options and am really careful about adding new ones.

@ckipp01
Copy link
Member

ckipp01 commented Aug 3, 2022

I'm not sure why the tests are failing 😓

Don't worry about the tests. There has been something funky going on lately that I'm unable to debug or haven't spent a lot of time on. As long as they are passing locally you're fine.

@antosha417
Copy link
Contributor Author

antosha417 commented Aug 3, 2022

On the top of my head both nerdtree and nvim-tree open folders and open files when you press either o or <cr>.

For me it sounds as natural as double tap on a folder opens a folder and double tap on a file opens this file.

@antosha417
Copy link
Contributor Author

I've also noticed this behavior: When I close some of the split buffers and my cursor automatically goes to tvp buffer than this jump to last window doesn't work and the file opens in tvp panel.

    -- Jump to the last window so this doesn't open up in the actual tvp panel
    vim.cmd([[wincmd p]])

The easiest way to reproduce it is to just close all the windows except of tvp panel. But it can happen on other circumstances.

So I'm thinking maybe in this case we want to create new window for it. With something like this:

    -- Jump to the last window so this doesn't open up in the actual tvp panel
    vim.cmd([[wincmd p]])
    if vim.api.nvim_get_current_buf() == self.bufnr then
      local alignment = "topleft"
      if config.panel_alignment == "left" then
        alignment = "botright"
      end
      vim.cmd(string.format("silent %s vertical new ", alignment))
    end

What do you think?

@antosha417
Copy link
Contributor Author

antosha417 commented Aug 3, 2022

Also a lot of classes in my projects have companion objects. And they look the same it tvp buffer because they have the same name.
I see that metals has support for icons in tvp. Do you maybe want me to add some icons?
What do you think about adding highlight groups to the output so it will be a bit easier to read and distinguish different elements?

Here for example we can easily see which AbstractActor is an object and which is a class:
Screen Shot 2022-08-04 at 0 14 41

Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, got around to playing around with this. I think it's non-invasive enough that this is totally fine. Thanks a lot for the PR! LGTM

@ckipp01 ckipp01 merged commit 9b9fad5 into scalameta:main Aug 6, 2022
@ckipp01
Copy link
Member

ckipp01 commented Aug 6, 2022

I see that metals has support for icons in tvp. Do you maybe want me to add some icons?
What do you think about adding highlight groups to the output so it will be a bit easier to read and distinguish different elements?

I would love having some icons.

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.

2 participants