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

File tree integration with nvim-tree and NERDTree #179

Merged
merged 4 commits into from
Nov 8, 2022

Conversation

zwhitchcox
Copy link
Contributor

This enables file tree integration with nvim-tree and NERDTree.

It is fairly difficult to get sessions and file tree browsers to play nicely together, but when you do, the result is spectacular.

As such, I thought I'd make it easier to integrate the two with a simple configuration option:

require 'nvim-tree'.setup {
  auto_session_enable_file_tree_integration = true,
}

This hopefully fixes:

And would be a solution for people struggling with sessions/file trees elsewhere as well:

README.md Outdated Show resolved Hide resolved
Copy link
Owner

@rmagatti rmagatti left a comment

Choose a reason for hiding this comment

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

Hey @zwhitchcox, thanks for the PR! I appreciate you trying to fix this!

However, I still have a pretty strong view on adding code into auto-session to support/interface with specific plugins, like I mention in this comment: #105 (comment)

Ultimately adding this code would mean I have to support a whole new set of functionality for interfacing with plugins I don't even personally use myself.

While I can certainly understand the appeal of "just fixing it here", the long-term support of this is what worries me.

@zwhitchcox
Copy link
Contributor Author

@rmagatti Hey, thanks for getting back to me so quickly. I actually think you're right. You don't want external plugin-specific code in your code, because then if they change something, it could break your code.

I had hoped there would be a file-tree generalized approach, but I can't see one. Oh well.

Instead, I think I can make a auto-session-nvim-tree plugin that can handle the nvim-tree-specific stuff.

Anyway, I took out all the external plugin-specific stuff and left in general bug fixes which prevent the "hook" solution from working. (causing session corruption, infinite loops, etc.)

Let me know if this looks good, and I'll squash the commits.

@zwhitchcox zwhitchcox force-pushed the file-tree-integration branch from b22064d to 014d905 Compare November 7, 2022 19:44

-- Deferring to avoid otherwise there are tresitter highlighting issues
vim.defer_fn(function()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This approach was causing an infinite loop. From the docs:

DirChanged After the current-directory was changed.
...
Non-recursive (event cannot trigger itself).

By triggering Autosession.AutoRestoreSession asynchronously, it caused an infinite loop where the opened buffer could change the global directory (somehow) after opening. So, I moved the asynchronous part to after the session was restored. It then runs filetype detect on all the open buffers which fixes the syntax highlighting error.

@@ -480,6 +480,19 @@ function AutoSession.RestoreSessionFromFile(session_file)
AutoSession.RestoreSession(string.format(AutoSession.get_root_dir() .. "%s.vim", session_file:gsub("/", "%%")))
end

--
-- Refresh syntax highlighting and file trees
local function post_restore_refresh()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what fixes the syntax highlighting. I moved it to after the restore session, so that it's always triggered. Although, I'm not sure if the problem exists other than in during the autocmd callbacks? So, it might be better to move it back there if not.

Copy link
Owner

Choose a reason for hiding this comment

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

Interesting, I hadn't noticed problems with this. But this should be fine anyway 👍

@zwhitchcox
Copy link
Contributor Author

Ok, I created a plugin auto-session-nvim-tree to your plugin, but those changes depend on this PR.

Please review the code and let me know what you think if you don't mind.

@zwhitchcox
Copy link
Contributor Author

Ok, I actually just realized there's a SessionLoadPost autocmd, so maybe I can use that which would make it compatible with all session loads, not just this library.

That's not really directly relevant to this PR, but more for the plugin.

SessionLoadPost			After loading the session file created using
				the [:mksession](https://neovim.io/doc/user/starting.html#%3Amksession) command.

This PR will still need to be merged though, because the aforementioned issue will still break, just wanted to let you know.

Copy link
Owner

@rmagatti rmagatti 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 putting the time into this! 🎉

@@ -480,6 +480,19 @@ function AutoSession.RestoreSessionFromFile(session_file)
AutoSession.RestoreSession(string.format(AutoSession.get_root_dir() .. "%s.vim", session_file:gsub("/", "%%")))
end

--
-- Refresh syntax highlighting and file trees
local function post_restore_refresh()
Copy link
Owner

Choose a reason for hiding this comment

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

Interesting, I hadn't noticed problems with this. But this should be fine anyway 👍

@rmagatti rmagatti merged commit 4af51e2 into rmagatti:main Nov 8, 2022
@zwhitchcox
Copy link
Contributor Author

zwhitchcox commented Nov 8, 2022

Yeah, happy to help!

So, upon further inspection, it looks like if you add localoptions to your session options:

vim.o.sessionoptions="blank,buffers,curdir,folds,tabpages,winsize,winpos,terminal,localoptions"

this will automatically restore syntax highlighting without needing to redetect the filetype. So, we could probably remove the filetype detect code and just make a note for people to save localoptions in the README. Do you want me to make a PR to this effect?

this also saves options like wrap and number

@rmagatti

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