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

Add -- to cp, mv and rm #1820

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Add -- to cp, mv and rm #1820

merged 1 commit into from
Feb 13, 2024

Conversation

KlzXS
Copy link
Collaborator

@KlzXS KlzXS commented Feb 13, 2024

I think these are all the instances where we use cp, mv and rm on user supplied paths, but I want someone else to confirm that as well.

Can someone who regularly uses the more "exotic" shells confirm if they support this syntax and if it's even necessary? They should, but let's make sure. Particularly in regard to quitcd.

I don't foresee any regressions, but I haven't done any testing as of yet.

Fixes #1818

@jarun jarun merged commit 4a9587a into jarun:master Feb 13, 2024
7 checks passed
@jarun
Copy link
Owner

jarun commented Feb 13, 2024

Thank you!

@@ -743,8 +743,8 @@ static const char * const envs[] = {

#define PROGRESS_CP "cpg -giRp"
#define PROGRESS_MV "mvg -gi"
static char cp[sizeof PROGRESS_CP] = "cp -iRp";
static char mv[sizeof PROGRESS_MV] = "mv -i";
static char cp[sizeof PROGRESS_CP] = "cp -iRp --";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm pretty sure this is broken now since the string exceeds the size of the buffer. IIRC the intention here was to have a buffer that is big enough to hold both the strings @KlzXS. And since the "progress" string was longer, it was used as a size.

Hopefully that made sense. If not feel free to ask questions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I hadn't noticed the other PR #1821. That should solve it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup! IMO this is a pretty hacky way to size the buffer, but since these strings ideally don't change often, I guess it's good enough.

If you notice anything else that stopped working or started working in a weird way point it out so that I can include it in the other PR. This one got merged a bit sooner than I expected.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't batch rename files which names start with dash '-'
3 participants