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

fix: Incorrect file copied message (#37) #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

miguelmoraperea
Copy link
Contributor

When copying a file outside the original directory (cwd) the file is not
copied, this is expected, but the user still gets the following message:

[file] has been copied!

This is incorrect, this commit fixes the issue by verifying the result
status from the copy operation and prints the appropriate message
'[file] has been copied!' or '[file] has not been copied' depending on
whether the operation succeeded or not.

When copying a file outside the original directory (cwd) the file is not
copied, this is expected, but the user still gets the following message:

  [file] has been copied!

This is incorrect, this commit fixes the issue by verifying the result
status from the copy operation and prints the appropriate message
'[file] has been copied!' or '[file] has not been copied' depending on
whether the operation succeeded or not.
@fdschmidt93
Copy link
Member

As a heads up, I'll try to look at this by mid next week :) It very much looks like a good change but also not urgent and I just want to have one clean pass whether typical scenarios are properly handled.

for key, value in pairs(res) do
if key.filename == destination then
local res_msg
if res[key] then
Copy link
Member

Choose a reason for hiding this comment

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

One thing that comes to mind here (referring to my other post), for instance are copying folders.

IIRC my plenary PR correctly, a folder would return a table of

{
  file_a.ext = bool,
  file_b.ext = bool,
  folder = { ... },
}

Hence, I think we'd be erroneously printing success, whenever one of the sub-files is copied improperly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out @fdschmidt93

I think you are onto something, but actually, we are not printing any message for copied folders.

I also noticed something interesting when copying files/folders, we are copying only the files/folders in the current dir and the first sub-dir down, all other deeper levels are recursively copied over, but we don't get a result for those file/directories after the first level down.

I need to take a closer look into this. I'll try to work on this sometime tomorrow.

@fdschmidt93
Copy link
Member

Some broader thoughts to (I will also) have think more about:

  • Should we rather print/notify failures rather than successes
  • (More broadly), should we use vim.notify as opposed to printing/echoing?

This goes a bit into the thought of making current multi-selections more visible, see discussion as per #41

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