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

rename 'remove' to 'exclude' when referring to candidate list #73

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

Conversation

eoghanmurray
Copy link

The word 'remove' is a synonym of 'delete', and nearly caused me a heart attack when I saw that it was apparently 'removing' the vast proportion of the input files (whereas the message meant that the vast proportion of input files were being removed from the files that are going to be further processed!)

  • I've changed the term from 'remove' to 'exclude'
  • the idea of there being a 'list' of files for consideration is really an internal implementation detail and shouldn't need to be exposed to the user.
  • Even in the README, the word remove was also being used as a synonym for delete: "I can now remove the one I consider a duplicate by hand if I want to"
  • the -removeidentinode option has been renamed to -excludeidentinode. Have left in support for previous option for backwards compatibility; hopefully I've done that right

…art attack when I saw that it was apparently 'removing' the vast proportion of the input files (whereas the message meant that the vast proportion of input files were being removed from the files that are going to be further processed!)

 - I've changed the term from 'remove' to 'exclude' when referencing the candidate list
 - The `-removeidentinode` option has been renamed to `-excludeidentinode`. Have left in support for previous option for backwards compatibility; hopefully I've done that right
 - In the README, the word remove is preserved when it is used as a synonym for delete: "I can now remove the one I consider a duplicate by hand if I want to"
 - For future consideration: the idea of there being a 'list' of files for consideration is really an internal implementation detail and shouldn't need to be exposed to the user IMO
@eoghanmurray
Copy link
Author

Sorry I don't understand the failing test output or whether it relates to the changes I've made?

@pauldreik
Copy link
Owner

Sorry for the late reply.
I understand that the word remove can be upsetting :-)

Internally, remove is ok to use in some places, as it has a well defined meaning in C++.

I will have to think about which parts of this PR I want to adopt. Thanks for the writeup and patch!

@eoghanmurray
Copy link
Author

Yep, feel free to just change the user visible output 👍

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