Skip to content

Update cp documentation#325

Closed
freitagbr wants to merge 1 commit intoshelljs:masterfrom
freitagbr:cp-documentation
Closed

Update cp documentation#325
freitagbr wants to merge 1 commit intoshelljs:masterfrom
freitagbr:cp-documentation

Conversation

@freitagbr
Copy link
Copy Markdown
Contributor

@freitagbr freitagbr commented Jan 30, 2016

Updates cp documentation to mention that files are not overwritten by default (#210).

nfischer
nfischer reviewed Jan 31, 2016
View reviewed changes
README.md Outdated
Copy link
Copy Markdown
Member

@nfischer nfischer Jan 31, 2016

Choose a reason for hiding this comment

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

s/unline/unlike

@nfischer
Copy link
Copy Markdown
Member

nfischer commented Jan 31, 2016

@freitagbr would you be interested in changing the default behavior? I think that would be preferable.

Keep the -f flag for legacy compatibility, but set -f to be the default. Then add the -n flag to perform the opposite behavior

@freitagbr
Copy link
Copy Markdown
Contributor Author

freitagbr commented Jan 31, 2016

@nfischer I would! I saw there was already a PR for adding -n (#192), can I go ahead and implement the feature on my own?

@nfischer
Copy link
Copy Markdown
Member

nfischer commented Jan 31, 2016

@freitagbr I think that's fine. Just make sure to address the remaining comments on that PR

@ariporad ariporad added docs high priority bash compat Compatibility issues with bash or POSIX behavior labels Jan 31, 2016
@ariporad ariporad added this to the v0.6.0 milestone Jan 31, 2016
@nfischer
Copy link
Copy Markdown
Member

nfischer commented Jan 31, 2016

@freitagbr please see the addition I added to parseOptions in #328

@nfischer nfischer mentioned this pull request Jan 31, 2016
Merged
@nfischer nfischer added blocked and removed blocked labels Jan 31, 2016
@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Feb 3, 2016

@nfischer, @freitagbr: What's the status on this? I'd really like to get this into v0.6.0.

@nfischer
Copy link
Copy Markdown
Member

nfischer commented Feb 3, 2016

Changing the default behavior should only be a couple of lines. I can open a PR if @freitagbr is too busy over the next few days.

@nfischer
Copy link
Copy Markdown
Member

nfischer commented Feb 3, 2016

@ariporad I'd rather change the default behavior to match unix than to update the docs to say our default behavior is wrong, especially since it should be a simple fix.

@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Feb 3, 2016

@nfischer: Oh, OK.

@nfischer nfischer mentioned this pull request Feb 4, 2016
Merged
@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Feb 4, 2016

@freitagbr: As much as I hate to reject a PR, it looks like this has been fixed by #336, so I have to close this. Sorry.

@ariporad ariporad closed this Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nfischer nfischer

Labels

bash compat Compatibility issues with bash or POSIX behavior docs high priority

Projects

None yet

Milestone

v0.6.0

Development

Successfully merging this pull request may close these issues.

3 participants

@freitagbr @nfischer @ariporad