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(#2630): file renames can now create directories #2657

Merged
merged 7 commits into from
Mar 3, 2024

Conversation

mohamedarish
Copy link
Contributor

I've implemented this feature.

I've just used the same code from create file as mentioned in the issue so as to not break any convention that was followed.

Any input is appreciated! :)

@alex-courtis
Copy link
Member

Many thanks, I'll get to this on the weekend.

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.

Please accept my apologies for not reviewing this earlier; this slipped off my list.

Unfortunately I'm not able to get anything working, including existing rename functionality, I think I'm Doing It Wrong. What OS / shell are you using?

Full rename testing using linux source:

  • CREDITS -> usr/CREDITS - usr/CREDITS directory created, file not moved
  • Makefile -> usr/Makefile - success reported, file not moved, should report: [NvimTree] Cannot rename Makefile -> Makefile: file already exists
  • Makefile -> foo/Makefile - foo/Makefile directory created, file not moved
  • README -> foo/bar/README - foo/bar/README created, file not moved
  • README -> READMEEEE - directory READMEEEE created, file not moved
  • usr/default_cpio_list -> usr/default_cpio_listtt - directory usr/default_cpio_listtt created, file not moved

Events appear to be correctly sent.

idx = idx + 1

local p = utils.path_remove_trailing(path)
if #path_to_create == 0 and vim.fn.has "win32" == 1 then
Copy link
Member

Choose a reason for hiding this comment

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

Please use utils.is_wsl or utils.is_windows

Copy link
Contributor Author

@mohamedarish mohamedarish Feb 11, 2024

Choose a reason for hiding this comment

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

I'm using wezterm terminal running zsh on macOS. I'll change the wsl, windows check on rename.
Should I change it in the create file because it is like what I've done right now?
I'll check into why this is failing :)

Copy link
Member

@alex-courtis alex-courtis Feb 11, 2024

Choose a reason for hiding this comment

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

I'm using wezterm terminal running zsh on macOS. I'll change the wsl, windows check on rename. Should I change it in the create file because it is like what I've done right now? I'll check into why this is failing :)

My apologies, I didn't realise it came from copy. That's fine, no need to change.

We need a big refactor / abstraction for that functionality, but Not Today.

I'd be very grateful for a future PR to do that :)

@mohamedarish
Copy link
Contributor Author

It's not fixed right now and shall reopen this PR after I've fixed the errors :)

@alex-courtis
Copy link
Member

It's not fixed right now and shall reopen this PR after I've fixed the errors :)

Hey no worries, take all the time you need.

I look forward to using this functionality...

@mohamedarish mohamedarish reopened this Feb 26, 2024
@mohamedarish mohamedarish reopened this Feb 26, 2024
@mohamedarish
Copy link
Contributor Author

Now the code does indeed work and has been tested.

Not many changes have been made since the closing of this PR but now it seems to work at least on my device.

My device:
Shell: 5.9 (x86_64-apple-darwin23.0)
Terminal: WezTerm 20240203-110809-5046fc22
NeoVim: v0.9.5
LuaJit: 2.1.1703358377
OS: MacOS Sonoma 14.3.1 (23D60)

@mohamedarish
Copy link
Contributor Author

mohamedarish commented Feb 26, 2024

I've tested it on a vm and the functionality seems to be working

VM:
Shell: GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)
Running via ssh
Neovim: NVIM v0.9.5
LuaJit: 2.1.1692716794
OS: Ubuntu 22.04.4 LTS

The renaming and directory creating functionality is working as intended and does not seem to create a directory instead of a file anymore

@alex-courtis
Copy link
Member

Thanks for reopening.

I'll get to review and test on the weekend.

@alex-courtis
Copy link
Member

alex-courtis commented Mar 3, 2024

Looking good:

Full rename u:

  • CREDITS -> usr/CREDITS
  • Makefile -> usr/Makefile - correct error reported
  • Makefile -> foo/Makefile
  • README -> foo/bar/README
  • README -> READMEEEE
  • usr/default_cpio_list -> usr/default_cpio_listtt

Rename r:

  • CREDITS -> usr/CREDITS
  • Makefile -> usr/Makefile - correct error reported
  • Makefile -> foo/Makefile
  • README -> foo/bar/README
  • README -> READMEEEE
  • usr/default_cpio_list -> usr/default_cpio_listtt
  • usr/include/headers_check.pl -> ../../foo/bar/headers_check.pl
  • usr/include/headers_check.pl -> ../../foo/bar/headers_check.pl.lll

Rename basename e:

  • .rustfmt.toml -> .rustfmtTTT
  • .rustfmt.toml -> usr/.rustfmt
  • .rustfmt.toml -> usr/.rustfmtTTT
  • .rustfmt.toml -> foo/bar/.rustfmt
  • .rustfmt.toml -> foo/bar/.rustfmtTTT

Rename omit filename <C-R>:

  • CREDITS -> usr/CREDITS
  • Makefile -> usr/Makefile - correct error reported
  • Makefile -> foo/Makefile
  • README -> foo/bar/README
  • README -> READMEEEE
  • usr/default_cpio_list -> usr/default_cpio_listtt

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 your contribution!

@alex-courtis alex-courtis changed the title fix(#2630): Allow creating directories when renaming files (behavior like create file) feat(#2630): file renames can now create directories Mar 3, 2024
@alex-courtis alex-courtis merged commit efafd73 into nvim-tree:master Mar 3, 2024
11 checks passed
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