Add ability to search by arbitrary metadata #568

vmiklos · 2021-02-14T15:13:23Z

Example:

~/git/git-bug/git-bug ls --metadata github-url=https://github.com/author/myproject/issues/42

Fixes #567.

MichaelMure

Looks like it's headed the right way (:+1:) but you are missing the query language part for now.

cache/filter.go

commands/ls.go

vmiklos · 2021-02-14T21:04:07Z

Looks like it's headed the right way (+1) but you are missing the query language part for now.

Right, that's only needed for the web part, correct? Unless you insist, I would be happy to leave that out from the current PR. I've updated the commit message to say it handles the cmdline piece.

Thanks.

MichaelMure · 2021-02-14T22:59:14Z

Right, that's only needed for the web part, correct? Unless you insist, I would be happy to leave that out from the current PR. I've updated the commit message to say it handles the cmdline piece.

No, it's also possible to use it with the CLI and the termui. I really don't want to have some features of the query language only supported in specific condition, so we should achieve parity.

vmiklos · 2021-02-15T17:16:08Z

git-bug ls metadata:github-url:https://github.com/author/myproject/issues/42

works for me now, could you please take a look? Thanks.

The commit message is unchanged, as the query language is working from the cmdline, but I didn't do anything for the web UI explicitly.

vmiklos · 2021-02-19T17:02:57Z

@MichaelMure can I help you in some way to get this reviewed or should I just wait more? (I can wait, I'm just a bit confused if you expect something from me or not). Thanks.

query/lexer.go

MichaelMure · 2021-02-19T17:26:54Z

Sorry, things takes time sometimes :)

Don't worry, your contribution is not lost.

MichaelMure

Alright, I think the last step is going to be completing the lexer/parser tests with those new cases.

MichaelMure · 2021-02-19T22:57:17Z

query/lexer.go

+		if len(split) == 2 {
+			tokens = append(tokens, newTokenKV(split[0], removeQuote(split[1])))
+		} else {
+			rest := strings.Join(split[2:], ":")


I'm not sure this should really accept more than two : in a token.

Do you have an alternative syntax in mind? I currently propose metadata:github-url:https://github.com/author/myproject/issues/42, but that means there will be 3 colons, even if the third is not interesting. I.e. if you ban > 2 colons, then colons are not allowed in metadata, which means URLs are not allowed in metadata, which would be sad.

Once we settle down on the syntax, I can take a look at the tests. :-) Thanks.

Hu, good point. Man, those formal grammar lessons that I barely listen to are quite far now ;)

After some idea shuffling, I'm thinking that the good solution might be to use quotes. It's already possible to do that: author:"René Descartes". So in your case it would be metadata:github-url:"https://github.com/author/myproject/issues/42".

Also, the command line parse behave in a similar way, no? Would git-bug ls --metadata github-url="https://github.com/author/myproject/issues/42" be accepted? Not that it would be required as long as the shell is happy, but some uniformity would be nice.

What do you think?

I like your proposal, and it may work for the webui, but the trouble is that the shell eats these quotes. So if I say meta:gh:"foo", I only get meta:gh:foo in go. You may require the user to type meta:gh:\"foo\", but that would be terrible usability.

Considering these, I would argue that either the current syntax is not that bad after all, or if you really would like some different syntax, then perhaps metadata:github-url=..., so it would be more obvious that the remaining characters are a value.

Unless we would go full json and allow something like metadata:{"github-url": "..."}, but again, typing that manually... ;-)

Which one would do you prefer?

So if I say meta:gh:"foo", I only get meta:gh:foo in go.

Is that really the shell or cobra? What do you get with meta:gh:"foo bar"?

--> I tested that, it's indeed go, but, go only unquote that and we get the args separated properly.

--> Actually, I had this problem before and to deal with it the ls command put the quotes back: https://github.com/MichaelMure/git-bug/blob/master/commands/ls.go#L93-L98

So, some shenanigan happen in the process but from the CLI side and parser side, it looks and behave as we expect it to.

then perhaps metadata:github-url=...

That could be an alternative way. The grammar would be only 1-tuples (for the full text search) and 2-tuples for the filters/sorting. No 3-tuples.

In the later case, we would still need the quotes IMHO, to make the grammar a bit more regular. Having : suddenly not being a separator because we already found two doesn't sounds like a great idea. What if we have 4 parameters next? It's unlikely but it would break the previously valid queries.

Considering this, this later syntax doesn't have much interest, no? What do you think?

Right. :-) One more thing I forgot is that the cmdline can always use ls --metadata github-url=https://github.com/author/myproject/issues/42, so using the query language is not a must. So in case we go with the metadata:github-url:"https://github.com/author/myproject/issues/42", that would be fine: it would be a good syntax (later) for the webui, and the cmdline can use metadata:github-url:\"https://github.com/author/myproject/issues/42\" if it really wants to use the query language for metadata.

Is this OK? If so, then I could have a go at updating the code to do this.

Example: ~/git/git-bug/git-bug ls --metadata github-url=https://github.com/author/myproject/issues/42 or ~/git/git-bug/git-bug ls metadata:github-url:\"https://github.com/author/myproject/issues/42\" Fixes the cmdline part of <MichaelMure#567>.

vmiklos · 2021-02-21T13:19:11Z

I've now implemented the foo:bar:"https://www.example.com/" syntax idea, also added tests. Please let me know if anything else is missing.

vmiklos · 2021-02-26T15:27:43Z

@MichaelMure can I help with anything to get this reviewed? Thanks.

MichaelMure · 2021-02-26T15:53:59Z

Nothing to do on your side, it's just that real life and day job don't give me much spare time.

MichaelMure · 2021-02-27T18:35:05Z

query/lexer.go

-		split := strings.Split(field, ":")
+		// Split using ':' as separator, but separators inside '"' don't count.
+		quoted := false
+		split := strings.FieldsFunc(field, func(r rune) bool {


I had this exact function to handle the quotes at some point, before writing this more complex/robust lexer. I don't remember exactly why, but IIRC this left a few edges cases open (maybe the unmatched quote?). I'll dig a bit to see if there is a better way to do that. Otherwise, this can be merged, because, well, it works.

MichaelMure · 2021-02-27T19:34:39Z

I ended up doing some refactoring/code golf to use the same split function to split on spaces and on :. I believe this cover more edge cases, and it's also simpler.

I think it's good to go now! Thanks for the help :)

vmiklos · 2021-02-27T22:31:39Z

Thanks for reviewing this! :-)

MichaelMure reviewed Feb 14, 2021

View reviewed changes

cache/filter.go Outdated Show resolved Hide resolved

cache/filter.go Outdated Show resolved Hide resolved

commands/ls.go Show resolved Hide resolved

MichaelMure marked this pull request as draft February 14, 2021 17:31

vmiklos force-pushed the search-metadata branch from 6fd211d to 2ada815 Compare February 14, 2021 21:00

vmiklos marked this pull request as ready for review February 14, 2021 21:04

vmiklos force-pushed the search-metadata branch from 2ada815 to 1a9cd4b Compare February 15, 2021 17:14

MichaelMure reviewed Feb 19, 2021

View reviewed changes

query/lexer.go Outdated Show resolved Hide resolved

vmiklos force-pushed the search-metadata branch from 1a9cd4b to 6d8e709 Compare February 19, 2021 21:14

MichaelMure reviewed Feb 19, 2021

View reviewed changes

vmiklos force-pushed the search-metadata branch from 6d8e709 to cb61245 Compare February 21, 2021 13:16

MichaelMure reviewed Feb 27, 2021

View reviewed changes

query: refactor to reuse the split function for both query and token

fab626a

MichaelMure merged commit 22cc4cc into MichaelMure:master Feb 27, 2021

vmiklos deleted the search-metadata branch February 27, 2021 22:31

GlancingMind mentioned this pull request Apr 29, 2021

Label naming #655

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to search by arbitrary metadata #568

Add ability to search by arbitrary metadata #568

vmiklos commented Feb 14, 2021

MichaelMure left a comment

vmiklos commented Feb 14, 2021

MichaelMure commented Feb 14, 2021

vmiklos commented Feb 15, 2021

vmiklos commented Feb 19, 2021

MichaelMure commented Feb 19, 2021

MichaelMure left a comment

MichaelMure Feb 19, 2021

vmiklos Feb 19, 2021

MichaelMure Feb 20, 2021

vmiklos Feb 20, 2021 •

edited

MichaelMure Feb 20, 2021

vmiklos Feb 20, 2021

vmiklos commented Feb 21, 2021

vmiklos commented Feb 26, 2021

MichaelMure commented Feb 26, 2021

MichaelMure Feb 27, 2021

MichaelMure commented Feb 27, 2021

vmiklos commented Feb 27, 2021

Add ability to search by arbitrary metadata #568

Add ability to search by arbitrary metadata #568

Conversation

vmiklos commented Feb 14, 2021

MichaelMure left a comment

Choose a reason for hiding this comment

vmiklos commented Feb 14, 2021

MichaelMure commented Feb 14, 2021

vmiklos commented Feb 15, 2021

vmiklos commented Feb 19, 2021

MichaelMure commented Feb 19, 2021

MichaelMure left a comment

Choose a reason for hiding this comment

MichaelMure Feb 19, 2021

Choose a reason for hiding this comment

vmiklos Feb 19, 2021

Choose a reason for hiding this comment

MichaelMure Feb 20, 2021

Choose a reason for hiding this comment

vmiklos Feb 20, 2021 • edited

Choose a reason for hiding this comment

MichaelMure Feb 20, 2021

Choose a reason for hiding this comment

vmiklos Feb 20, 2021

Choose a reason for hiding this comment

vmiklos commented Feb 21, 2021

vmiklos commented Feb 26, 2021

MichaelMure commented Feb 26, 2021

MichaelMure Feb 27, 2021

Choose a reason for hiding this comment

MichaelMure commented Feb 27, 2021

vmiklos commented Feb 27, 2021

vmiklos Feb 20, 2021 •

edited