Skip to content

feat: add same_level config option #522

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

Merged
merged 7 commits into from
Jan 12, 2023

Conversation

zenoli
Copy link
Contributor

@zenoli zenoli commented Sep 15, 2022

Hi, first of all, thanks for this amazing plugin!
This is my first contribution and I kept the PR minimal because I want to first assess whether this feature is even wanted before diving deeper into the setup.

I propose to add a same_level boolean config option to control the behaviour on how created/pasted/moved files/directories get inserted into the tree:

The behaviour is only affected if the cursor during creation/pasting/moving is on a directory node. Currently, the items get inserted inside the directory under cursor. I personally find it more intuitive if they get inserted on the same level (i.e. "as siblings") to the directory under cursor, much like it behaves currently when the cursor is on a file.

If you think this feature is worth continuing to work on let me know. If no, feel free to decline the PR. If yes, I would also like to add the same_level flag withing the config table of the specific commands (i.e. next to config.show_path which would take precedence over the global same_level options.

@cseickel
Copy link
Contributor

This seems like a reasonable addition. I do think the option name could be clearer.

Maybe we can call the option something like "target" or "destination" and make the option value "child" by default or "sibling" for the behavior you want.

Or to keep it a boolean, make it "always_paste_as_sibling" or something else descriptive.

@cseickel
Copy link
Contributor

I would also like to add the same_level flag withing the config table of the specific commands (i.e. next to config.show_path which would take precedence over the global same_level options.

In this case, you can actually have both and the new option would affect what the default path is when "show_path" is used.

@nyngwang
Copy link

nyngwang commented Sep 20, 2022

@zenoli @cseickel Considering the usefulness, I will say yes: I once wanted to create many "sibling" folders. but then I will need to manually move the cursor onto any non-folder node under the current parent. But it is possible that there is no non-folder node under the current parent. So, this PR will be useful for me when it is merged.

Regarding naming: How about parent default to same(or current) and can be set to cursor, i.e. the node under the cursor will become the parent of the created node.

@zenoli
Copy link
Contributor Author

zenoli commented Sep 23, 2022

Awesome, thanks for the feedback!
I won't have time to tackle this the next days as I'm in vacation. I'll get back to you with some clarifying questions once I'm working on it.

@zenoli zenoli force-pushed the same-level-config-option branch from 2af113c to 11870a6 Compare October 20, 2022 13:35
@zenoli
Copy link
Contributor Author

zenoli commented Oct 20, 2022

Hey :-)
I finally got the time to make some progress. I implemented the override behaviour for the command-specific configs.
However, while testing it, I noticed some issues with empty folders: With the "same_level" flag set, it becomes impossible to insert into empty directories. I compared the behaviour to nvim-tree and they do it like this:
If the folder under cursor is expanded, paste it as children otherwise as sibling. I would really like to mimick this, because it feels very intuitive. The issue issue with neo-tree is, that there seems to be no notion of "expanded/collapsed" for empty directories.

I assume this is becausenui.nvim does not alllow for empty directories to be expanded:

---@return boolean is_updated
function TreeNode:expand()
  if self:has_children() and not self:is_expanded() then
    self._is_expanded = true
    return true
  end
  return false
end

Am I correct with this assumption?
How hard would it be to implement the collapsed/expanded state for empty dirs? If I could get some pointers I might do it myself.

And on another note:
Currently I would prefer the config name "insert_as" with possible values of "child" (default) and "sibling. What do you think?

@cseickel
Copy link
Contributor

The issue issue with neo-tree is, that there seems to be no notion of "expanded/collapsed" for empty directories.

True, but neo-tree does have the ability to distinguish this. It is used to display a different icon for known empty directories:

if node.loaded and not node:has_children() then -- empty folder

if node.loaded and not node:has_children() then
icon = config.folder_empty or config.folder_open or "-"
elseif node:is_expanded() then
icon = config.folder_open or "-"
else
icon = config.folder_closed or "+"
end

Currently I would prefer the config name "insert_as" with possible values of "child" (default) and "sibling. What do you think?

That's sounds reasonable.

@cseickel
Copy link
Contributor

@zenoli This seems to have a lot of changes related to the other PR (#600) that was just merged. Does it need another rebase?

@zenoli zenoli force-pushed the same-level-config-option branch from a2491da to d22fd6a Compare January 12, 2023 09:34
@zenoli
Copy link
Contributor Author

zenoli commented Jan 12, 2023

@cseickel yes, I constantly rebased this branch onto #600 because they were somewhat related and I wanted to test it locally so I could work on both issues simultaneosly.
I rebased the relevant commits to main so it should be fine now.

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.

3 participants