preservim / nerdtree

A tree explorer plugin for vim.
Do What The F*ck You Want To Public License
19.56k stars 1.44k forks source link

Warn invalid files #1365

Closed rzvxa closed 1 year ago

rzvxa commented 1 year ago

Description of Changes

Closes #1274


New Version Info

Fixed issues with the original PR

Author's Instructions

rzvxa commented 1 year ago

Thanks for helping out with existing PRs. I would like to make sure we keep credit for the original author though. Somehow you've rewritten the original commits instead of keeping them. Can you please rebase this on the other PR commits so the original author meta data is kept, and then your commit appended to the list? At most I would squash them and use Co-authored-by headers, but separate commits is fine too. They just need to be at least partial credit to the OP.

Initially, I tried to rebase but Git complained about broken references as there were some removed commits here and there, So I just used cherry-picking to patch my fork, I can go back and get the original commit in, But it would be much easier to add credit on the merge commit. If you don't want to do it this way, Tell me so I can go ahead and rebase it for real this time

alerque commented 1 year ago

Initially, I tried to rebase but Git complained about broken references as there were some removed commits here and there, So I just used cherry-picking to patch my fork, I can go back and get the original commit in, But it would be much easier to add credit on the merge commit.

No, there are no removed commits between that PR and this one. The are only 4 commits apart in the tree, and it would be impossible for the other PR to even be valid if it wasn't based on this repository. I didn't have any trouble rebasing your commits here onto the original PR.

If you don't want to do it this way, Tell me so I can go ahead and rebase it for real this time

I just did, and also merged one of the commits. You can give it a look here and if all looks okay to you I think we can merge.

rzvxa commented 1 year ago

@alerque Thanks for merging the initial PR, Looks good to me, Let's go ahead and merge this.