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

BCP-96: Add Glob Patterns Support #96

Merged
merged 6 commits into from
Nov 8, 2021
Merged

Conversation

JinnyYi
Copy link
Contributor

@JinnyYi JinnyYi commented Nov 4, 2021

ref: #77

@JinnyYi JinnyYi changed the title proposal: Add Glob Pattern Support in cp BCP-96: Add Glob Pattern Support in cp Nov 4, 2021
@Xuanwo
Copy link
Contributor

Xuanwo commented Nov 5, 2021

Glob support sounds like can also be used in mv, ls and rm. Maybe our BCP should support them in general? For example:

byctl cp *.png example:/

@JinnyYi
Copy link
Contributor Author

JinnyYi commented Nov 5, 2021

Glob support sounds like can also be used in mv, ls and rm. Maybe our BCP should support them in general?

Yes. I'll update it.

byctl cp *.png example:/

It looks more friendly then --exclude and --include. In addition, --exclude and --include do not support patterns containing directory info, e.g., --include "/usr/fo*/test/*.jpg".

@JinnyYi JinnyYi changed the title BCP-96: Add Glob Pattern Support in cp BCP-96: Add Glob Patterns Support Nov 5, 2021
@Xuanwo
Copy link
Contributor

Xuanwo commented Nov 5, 2021

It looks more friendly then --exclude and --include. In addition, --exclude and --include do not support patterns containing directory info, e.g., --include "/usr/fo*/test/*.jpg".

How about supporting byctl cp *.png example:/ only and leave exclude and include to implement later?

@JinnyYi
Copy link
Contributor Author

JinnyYi commented Nov 5, 2021

How about supporting byctl cp *.png example:/ only and leave exclude and include to implement later?

I‘ve moved exclude and include to the Alternative Way, does this section need to be removed?

In addition, do we expect to support only * pattern, or do we support the multiple patterns listed in the latest commit?

@Xuanwo
Copy link
Contributor

Xuanwo commented Nov 5, 2021

I‘ve moved exclude and include to the Alternative Way, does this section need to be removed?

LGTM, we don't need to remove them.

In addition, do we expect to support only * pattern, or do we support the multiple patterns listed in the latest commit?

Maybe we can use https://github.com/gobwas/glob.

GitHub
Go glob. Contribute to gobwas/glob development by creating an account on GitHub.

@JinnyYi
Copy link
Contributor Author

JinnyYi commented Nov 5, 2021

Maybe we can use https://github.com/gobwas/glob.

Got it. filepath.Match() is slightly faster then gobwas/glob from my test, but filepath.Match() doesn't support { }.

GitHub
Go glob. Contribute to gobwas/glob development by creating an account on GitHub.

@Xuanwo
Copy link
Contributor

Xuanwo commented Nov 8, 2021

Got it. filepath.Match() is slightly faster then gobwas/glob from my test, but filepath.Match() doesn't support { }.

I think we need to support ** which filepath.Match doesn't support.

@Xuanwo
Copy link
Contributor

Xuanwo commented Nov 8, 2021

Mostly LGTM, let's create a tracking issue about this BCP.

@Xuanwo Xuanwo merged commit 4304d39 into beyondstorage:master Nov 8, 2021
@JinnyYi JinnyYi deleted the issue-77 branch November 16, 2021 05:43
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