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: setup hooks for sources to provide custom stats for nodes #1121

Merged
merged 2 commits into from
Aug 27, 2023

Conversation

cseickel
Copy link
Contributor

@cseickel cseickel commented Aug 27, 2023

This tweaks how file stats are pulled so that sources (internal or external) can provide their own. This is only necessary if the node does not represent an object in the local filesystem.

See here for an example the API to fulfill:
https://github.com/nvim-neo-tree/example-source/blob/1be4d91d94875501cb4db1a61aa5155b6cd654bc/lua/example/init.lua#L19-L49

@cseickel
Copy link
Contributor Author

You know @miversen33, I think I have a better idea for this, let me know what you think.

What if instead of a source having to providing a single get_node_stat function, we instead had the source add a new field to the items sent to renderer.showNodes() called stat_source which is a string that indicates where to get stats for this node. You could then register any number of "stat sources" in your setup() function or just the root of your module with something like utils.register_stat_source(name, fn).

There are several things I like about this method:

  1. We go back to only needing the node object itself as an argument to the original utils.get_stat(node) function.
  2. You can technically have any number of stat providers in a source.
  3. External plugins are not forced into any particular code organization for this.
  4. A stat provider could be external to a source, there can even be plugins that just provide (or override) stat providers.

@miversen33
Copy link
Collaborator

With your new idea, would the end flow be that when a user requests stat results for a node, that Neo-tree would call whatever source is provided with the most recent showNodes command? What happens if at some point, a source decides to provide a different source for stat? How would that be managed (are you thinking there would just be a variable that is applied to the state that is overwritten?) I don't see that situation happening often (if ever) but it could since this would allow a source to dynamically change the stat provider on-the-fly.

add a new field to the items sent to renderer.showNodes() called stat_source which is a string that indicates where to get stats for this node.

I understand the gist of this but I don't know that I fully understand how it would be implemented as a source provider. Would the idea be that I call render.showNodes(some_node_list, some_stat_func)? You mention that we would have to set up some_stat_func in our setup, could you expand on that a bit? Would the idea be that we add some function we have in scope to the setup config?

local function do_stat_thingy(state, node)
    print("Fetching Stat for node", node:get_id())
end

M.setup(config, global_conf)
    config.do_stat_thingy = do_stat_thing
end

Something like this?


I think either idea is fine, though I do like the idea of it being easier to maintain as a developer (so you in this case). If you feel that your suggestion would be easier for you to create/maintain/add to, I say we go for it :) I just want to understand how I as a source developer would utilize the new method for stat

@cseickel
Copy link
Contributor Author

I don't think you are quite getting what I was trying to explain. What I meant was that you would do two things. First, in your setup function, register the stat provider(s):

local function get_stat_for_docker(node)
    print("Fetching Stat for node", node:get_id())
end

local function get_stat_for_ssh(node)
    print("Fetching Stat for node", node:get_id())
end

M.setup(config, global_conf)
  local utils = require("neo-tree.utils")
  utils.register_stat_provider("netman-docker", get_stat_for_docker)
  utils.register_stat_provider("netman-ssh", get_stat_for_ssh)
end

I don't know if you really need multiple functions or just one, but you can provide as many as you need.

Then, when you provide the data to the renderer, you just add an extra field on every item that defines the name of the stat provider to use:


  local items = {}
  table.insert(items, {
    path = "user@host:/home/user/myfile.txt",
    type = "file",
    stat_provider = "netman-ssh"
  })
  renderer.showNodes(items, state)

Then inside neo-tree core, we may have any number of stat providers registered and every node will specify by name what stat provider is appropriate for itself. The existing utils.get_stat(node) function will perform the lookup into the registered providers if needed, or fallback to the local libuv stat if node.stat_provider == nil.

@miversen33
Copy link
Collaborator

I'm tracking. Perfect, I think that will work great :)

What will be the behavior shown to the end user when they attempt to view stat results on a node that doesn't have a stat provider? Say they try to view stat results on the git source, where stat doesn't make any sense to implement. Is an error/warning thrown? Would the end user see "nothing" happen?

An additional architectural question, you have some new columns that are on the filesystem source for displaying stat information (merged with #1107). I notice in your code that you call the get_stat_node function for each node as its rendered. I assume this also applies to redraws yes?

My biggest concern (and this is purely a concern with my own interface with neo-tree as opposed to neo-tree in general) is that calling stat a ton of times will get expensive. Especially if done remotely. I have some caching stuff I am already doing so its not a huge deal for me, but I do want to verify that this get_stat_node call (or whatever the end name is) is indeed called on redraws as well as showNode calls.

@cseickel
Copy link
Contributor Author

What will be the behavior shown to the end user when they attempt to view stat results on a node that doesn't have a stat provider?

It just renders -.

Say they try to view stat results on the git source, where stat doesn't make any sense to implement.

Actually, those are just files and directories so it makes perfect sense to show those stats.

I notice in your code that you call the get_stat_node function for each node as its rendered. I assume this also applies to redraws yes?

The utils.get_stat function is already catching the result on the node itself, so the "stat_provider" function will not be called for redraws.

@cseickel cseickel enabled auto-merge (squash) August 27, 2023 19:38
@cseickel cseickel disabled auto-merge August 27, 2023 19:38
@cseickel cseickel merged commit 2c99276 into main Aug 27, 2023
2 checks passed
@cseickel cseickel deleted the allow-custom-get-stat branch August 27, 2023 19:39
@cseickel
Copy link
Contributor Author

@miversen33 this is ready to use. See the example source project for instructions: https://github.com/nvim-neo-tree/example-source#custom-stat-provider

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.

None yet

2 participants