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

Sort grammar rewrite and bugfix with multiple fields #65

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

51-code
Copy link
Contributor

@51-code 51-code commented Aug 12, 2024

Fixes #49 and #64 .

Rewrote sort command's grammar to allow multiple fields with a comma delimiter.
Grammar was refactored because it didn't support having the optional parameters anywhere in the query. So, the main change is the main grammar rule in sort which now allows having the parameters in any order and as many times as wanted. Now it is more or less left for PTH-10 to check that the parameters are given just once etc.

Added a new grammar rule for the plus sign: differentiated it from the minus for easier visiting in PTH-10.

Wrote unit tests for sort command, there weren't any before.

@51-code 51-code added bug Something isn't working feature Existing feature labels Aug 12, 2024
@51-code 51-code requested a review from eemhu August 12, 2024 12:07
@51-code 51-code linked an issue Aug 12, 2024 that may be closed by this pull request
Copy link

@eemhu eemhu left a comment

Choose a reason for hiding this comment

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

Add test for +/- with sort function case
e.g. -num(a) +str(b)

@51-code 51-code requested a review from eemhu August 13, 2024 10:40
Copy link

@eemhu eemhu left a comment

Choose a reason for hiding this comment

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

approved

@kortemik kortemik merged commit b02551e into teragrep:main Aug 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature Existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create unit tests for sort command Sort command only takes up to two fields as parameter
4 participants