Skip to content

Conversation

@bftelman
Copy link
Contributor

@bftelman bftelman commented Jan 5, 2023

I have a suggestion to give users the option to disable the prompt when deleting a file. For example, when working in small folders, there is no particular need to double-check the file you are deleting

Copy link
Collaborator

@gegoune gegoune left a comment

Choose a reason for hiding this comment

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

We should try to avoid adding new options if at all possible. If the is highly desirable I would like that option name to be changed to something like:

ui = {
  confirm = {
    node_deletion = true,
  }
}

(node can actually be called file, because everything is a file :)

Would you be willing to add other confirmations as well?

@alex-courtis

@alex-courtis
Copy link
Member

Would you be willing to add other confirmations as well?

Yes. Some of these are annoying, but it would be good to be selective.

How about we add an enable to turn them all off?

ui = {
  confirm = {
    enable = true,
    node_deletion = true,
    paste = true,
    trash = true,
    create = true,

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Many thanks for taking the initiative on this UX improvement.

Looks like a straightforward refactor.

If you are motivated to make the same change for other operations, and implement ui.confirm.enable I would be most grateful.

end

local DEFAULT_OPTS = { -- BEGIN_DEFAULT_OPTS
file_deletion_confirmation = true,
Copy link
Member

Choose a reason for hiding this comment

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

Please refactor this to ui.confirm.remove.

@bftelman
Copy link
Contributor Author

bftelman commented Jan 7, 2023

Made refactors as suggested

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Remove works beautifully, trash fails at cf36d35 with:

E5108: Error executing lua: ...cker/start/amamdemous/lua/nvim-tree/actions/fs/trash.lua:32: attempt to index field 'trash' (a nil value)
stack traceback:
        ...cker/start/amamdemous/lua/nvim-tree/actions/fs/trash.lua:32: in function 'handle_tree_actions'
        ...cker/start/amamdemous/lua/nvim-tree/actions/dispatch.lua:123: in function 'dispatch'
        ...k/packer/start/amamdemous/lua/nvim-tree/actions/init.lua:273: in function <...k/packer/start/amamdemous/lua/nvim-tree/actions/init.lua:272>

remove = true,
trash = true,
},
},
Copy link
Member

Choose a reason for hiding this comment

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

Next time please remember to update help as per CONTRIBUTING

Added doc for this section and renamed to match the action.

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Works nicely, thank you for your contribution.

@alex-courtis alex-courtis merged commit ccb6d8a into nvim-tree:master Jan 8, 2023
alex-courtis added a commit that referenced this pull request Jan 9, 2023
…uire_confirm (#1887)

* fix(#1878): nvim frozen on no name buffer when modified.enable (#1879)

* feat(api): add api.config.mappings.active, api.config.mappings.default (#1876)

* feat(api): add config.mappings.current and config.mappings.default

* feat(api): add config.mappings.current and config.mappings.default

* feat(api): add config.mappings.current and config.mappings.default

* Implement turning off y/n prompt for file deletion

* Refactor different prompt configs to single ui table

* Remove unused fields

* add ui.confirm doc, format/tidy

* trash.require_confirm -> ui.confirm.trash

* Fix nil value trash field

Co-authored-by: Richard Li <[email protected]>
Co-authored-by: Telman Babayev <[email protected]>
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