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

Implement add/1 #3144

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Implement add/1 #3144

merged 1 commit into from
Aug 20, 2024

Conversation

myaaaaaaaaa
Copy link
Contributor

@myaaaaaaaaa myaaaaaaaaa commented Jun 24, 2024

Improves the ergonomics of add by avoiding the need for wrapper arrays. Assignments should be easier now too:

-... | .field = ([ ... ] | add) | ...
+... | .field = add(...) | ...

Related to #2595

@wader
Copy link
Member

wader commented Jun 24, 2024

I like it. What do other ppl say? from the original issue there was also talk about generator variants of min/max, add those too?

@myaaaaaaaaa myaaaaaaaaa changed the title Implement add(generator) Implement generator variants of add/min/max Jun 24, 2024
@myaaaaaaaaa
Copy link
Contributor Author

from the original issue there was also talk about generator variants of min/max, add those too?

Done

src/builtin.jq Outdated Show resolved Hide resolved
@pkoppstein
Copy link
Contributor

Adding a stream-oriented add is long-overdue, so my hope is that its introduction will not be delayed by potentially contentious issues.

Adding stream-oriented versions of min, max, min_by, and max_by would be fine, but it seems to me that existing implementations in C should be retained, both for the sake of efficiency and perhaps to facilitate future C-oriented development.

Maybe the best would be to have one narrowly focused PR for add; and one or more separate ones for the rest.

@myaaaaaaaaa
Copy link
Contributor Author

myaaaaaaaaa commented Jun 25, 2024

It looks like min/1 and max/1 have been attempted before, and met the same concerns regarding jq vs C performance:

Perhaps it would be prudent to discuss the best approach for min/1 and max/1 in #2595 first?

For now, I'll revert this PR to focus solely on add/1

@myaaaaaaaaa myaaaaaaaaa changed the title Implement generator variants of add/min/max Implement add/1 Jun 25, 2024
@myaaaaaaaaa
Copy link
Contributor Author

Friendly ping

Copy link
Contributor

@itchyny itchyny left a comment

Choose a reason for hiding this comment

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

Thank you!

@itchyny itchyny merged commit 0b5ae30 into jqlang:master Aug 20, 2024
29 checks passed
@myaaaaaaaaa myaaaaaaaaa deleted the addgen branch August 20, 2024 16:17
@itchyny itchyny added this to the 1.8 release milestone Aug 21, 2024
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.

5 participants