Add support for expansion of direct children of query keys #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
issmirnov
requested changes
May 22, 2022
cmd/zap/text_test.go
Outdated
| So(res.String(), ShouldEqual, "https://github.com/search?q=issmirnov") | ||
| }) | ||
| }) | ||
| Convey("Given 'g/s/me.z'", t, func() { |
There was a problem hiding this comment.
I'm guessing this is a typo - did you mean to put a slash instead between me and z ?
|
Hi Chris! Thank you for the PR, this makes a lot of sense and looks good to me. Found one tiny typo in the test case, other than that it looks good to go. I'm glad that the tool is useful to you! |
issmirnov
approved these changes
May 23, 2022
|
v1.3.0 has been packaged and deployed, please enjoy! https://github.com/issmirnov/zap/releases/tag/v1.3.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey Vania, hope things are well! Been using Zap consistently on all my machines for the last few years and for the most part it's been smooth and "just worked". One thing that's stuck out is the lack of support for expanding elements after a query, which would come in handy with things like Gerrit which expose an endpoint for viewing open PRs on a repo at
/q/project:<project>. Right now I have to fully type out the project name, but with this change I can add shortcuts for those.If merged as-is, this would technically be a breaking change. For example, with a config of:
the expansion for the URL
g/s/thing/akwould go fromgithub.com/search?q=thing/apache/kafkatogithub.com/search?q=thing/ak, and forg/s/ak, would go fromgithub.com/search?q=aktogithub.com/search?q=apache/kafka.I'd argue that since this is addressing a bug and it's unlikely that people are relying on the existing expansion behavior for keys underneath queries, it'd be fine to make this change. But if not, we can consider introducing a separate key type like
query2(awful name, would need to change) that would enable this behavior while preserving the existing behavior forquery, or adding a global flag to the config file or the CLI that enables/disables this new behavior.