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

Added globfilter command #1650

Merged
merged 4 commits into from
Apr 7, 2024
Merged

Conversation

Catalyn45
Copy link
Contributor

@Catalyn45 Catalyn45 commented Mar 19, 2024

fixes: #1649, Added a new globfilter option which is verry similar to globsearch but only apply to the filter command.

I only added documentation in doc.md because I think the rest of the files will be auto-generated on the next release (or the one after the next one)? Or should I run gen/doc-with-docker.sh myself? Please let me know.

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, overall the code looks like how I would have implemented it myself.

I have left a few minor comments, if you could fix them that would be great.

diacritics_test.go Outdated Show resolved Hide resolved
doc.md Outdated Show resolved Hide resolved
eval.go Outdated Show resolved Hide resolved
opts.go Show resolved Hide resolved
@joelim-work
Copy link
Collaborator

Regarding the documentation, if you have Docker setup then you can just run gen/doc-with-docker.sh. It is not mandatory, we will run it anyway just before a new release is made just in case, but it does make things easier as the diffs will be smaller if updated incrementally rather than once in bulk.

Also please add Fixes #1649 to the PR description to link it properly, so it will close the corresponding issue when the PR is merged.

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the changes once again.

@gokcehan gokcehan merged commit f507b61 into gokcehan:master Apr 7, 2024
4 checks passed
@joelim-work
Copy link
Collaborator

joelim-work commented Jun 24, 2024

Marking this is as a breaking change since prior to this globsearch was used to enable globs when using filter, but now it is globfilter that is required to be set instead.

@joelim-work joelim-work added this to the r33 milestone Jun 24, 2024
@joelim-work joelim-work added the breaking Pull requests that introduce breaking changes label Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Pull requests that introduce breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: sepparate glob option for filter command
3 participants