fix(#2395): marks.bulk.move defaults to directory at cursor

[fastndead]

fixes #2395

Hey! In this PR I attempt to implement #2395 feature request.
The logic for bulk move is rendered unusable for me for the most part because every time I want to perform a bulk move I need to write the whole path from the cwd to the destination. when working with files nested deep inside the file structure this can be frustrating and I almost always retreat to opening the files in the Finder and moving them via system UI. This PR is my attempt to solve this problem.

This is not a breaking change. I add a new option to the configuration, leaving the default behaviour untouched, so the new functionality can be accessed on an opt in basis.

[alex-courtis]

Apologies for the delay in review.

This works nicely:

  bulk_move_get_default_path = function()
    return string.format("%s/foo", vim.fn.getcwd())
  end,

[alex-courtis]

Rather than adding another option can we please add this to the API instead - it will be non-breaking.

marks.bulk.move({opts})                            *nvim-tree-api.marks.bulk.move()*
    Prompts for a directory to move all marked nodes into.

    Options: ~
      • {prompt}   (fun(absolute_path: string): string) takes the absolute path of the node under the cursor, returns the absolute path for the prompt

This change will be complete and releasable.

We can add further functionality to default the prompt to the node under the cursor in a later PR, or this PR if you are motivated.

[fastndead]

@alex-courtis I didn't like appending the options of the whole plugin either, i agree with you that adding the options to the bulk_move function would be a better move.

I guess I'm just a little confused on how would user use the updated feature. Would they have to remap bmv from using bulk.move with opts passed? sorry in advance if that's a noob question

[fastndead]

@alex-courtis i removed global option and added the option to the bulk move function, could you please take a look. Hope I got your idea correctly

[alex-courtis]

This works nicely.

local function my_bulk_move()
  api.marks.bulk.move({
    prompt = function(path)
      return string.format("%s", path)
    end,
  })
end

---

  vim.keymap.set('n', 'bmv',   my_bulk_move,                 opts('Move Bookmarked'))

[alex-courtis]

get_node_at_cursor must be nil checked

[alex-courtis]

I guess I'm just a little confused on how would user use the updated feature. Would they have to remap bmv from using bulk.move with opts passed? sorry in advance if that's a noob question

Yes, as per the trivial case above.

I am also confused: what is the use case for this? Do you have one?

If we can't find one we can simplify this. I'm happy to class the default prompt change as a bug fix.

[fastndead]

@alex-courtis well, my use case is moving the files to the place where the cursor is as default and that's it. As far as simplification goes i would say it would be simpler to have an option that would be a boolean that would enable the bulk_move default prompt to be a node under the cursor and that's it. I don't really see any other use cases for it so maybe a whole custom callback might be an overkill.

I honestly don't know if people are used to the default prompt being a cwd, so i would not take it upon myself to introduce a breaking change making node_under_cursor a default, but maybe you have some insight, if you think making node_under_cursor default is a good idea i would agree with you

i will make it boolean with default being the way it is in the prod (no breaking changes), but please let me know if you think we should actually change default to being node_under_cursor

[alex-courtis]

I honestly don't know if people are used to the default prompt being a cwd, so i would not take it upon myself to introduce a breaking change making node_under_cursor a default, but maybe you have some insight, if you think making node_under_cursor default is a good idea i would agree with you

That is completely reasonable. I don't imagine anyone is particularly happy with cwd. Let's just default it and release as fix(#2395): marks.bulk.move defaults to directory at cursor

Having an optional path and not breaking sounded reasonable in the issue discussion, however not so now, once we've actually built and played with it.

[alex-courtis]

Enhancement, no use case:

• path parameter OR
• callback parameter

Enhancement, paste parity but YAGNI:

• boolean prompt parameter, default true OR
• ui.confirm.bulk.move

Let's just fix the default; we can add more functionality only if requested.

[fastndead]

Let's just default it and release as fix(#2395): marks.bulk.move defaults to directory at cursor

sounds great! did that, please take a look

[alex-courtis]

Really close, it's working nicely... we just need to use the file node's directory when moving e.g. should move to virt

We can do whatever copy/paste does to find the containing directory.

[fastndead]

@alex-courtis you're totally right, I missed it. Now it should work as expected

[alex-courtis]

Looking good... one more little one:

bmv to root:

E5108: Error executing lua: ...packer/start/fastndead/lua/nvim-tree/marks/bulk-move.lua:22: attempt to index field 'parent' (a nil value)
stack traceback:
        ...packer/start/fastndead/lua/nvim-tree/marks/bulk-move.lua:22: in function <...packer/start/fastndead/lua/nvim-tree/marks/bulk-move.lua:12>

[fastndead]

@alex-courtis oh! this doesn't seem to be reproducible on Mac, that's why i didn't notice that, thanks for pointing it out!

0fc5032

i added a check for node_at_cursor.parent before indexing it so the issues should disappear now

[alex-courtis]

Love your work @fastndead