Skip to content

mr_list: Support search#560

Merged
bmeneg merged 2 commits intozaquestion:masterfrom
fmuellner:mr-search
Jan 22, 2021
Merged

mr_list: Support search#560
bmeneg merged 2 commits intozaquestion:masterfrom
fmuellner:mr-search

Conversation

@fmuellner
Copy link
Copy Markdown
Contributor

Search is as useful for merge requests as for issues, so support
the same option as lab issue list.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 13, 2021

Codecov Report

Merging #560 (8251fd6) into master (7c2c165) will decrease coverage by 0.05%.
The diff coverage is 46.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #560      +/-   ##
==========================================
- Coverage   58.30%   58.25%   -0.06%     
==========================================
  Files          64       64              
  Lines        4197     4211      +14     
==========================================
+ Hits         2447     2453       +6     
- Misses       1516     1522       +6     
- Partials      234      236       +2     
Impacted Files Coverage Δ
cmd/issue_list.go 78.94% <42.85%> (-5.06%) ⬇️
cmd/mr_list.go 72.34% <50.00%> (-2.38%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c2c165...8251fd6. Read the comment docs.

@bmeneg
Copy link
Copy Markdown
Collaborator

bmeneg commented Jan 13, 2021

@fmuellner do you think it would be useful a "--full-match" flag for adding double-quotes in the terms and forcing that "exact" (it still is case insensitive) match?
It would basically be the same as lab issue search "\"search terms\"".

@fmuellner
Copy link
Copy Markdown
Contributor Author

do you think it would be useful a "--full-match" flag for adding double-quotes in the terms and forcing that "exact" (it still is case insensitive) match?
It would basically be the same as lab issue search "\"search terms\"".

Do you mean to both lab mr list and lab issue list? IMHO those two commands should be as consistent as possible.

That said, a flag does sound more convenient than escaping quotes on the command line. I'd prefer --exact-match over --full-match though - the latter suggests something like full-text search (i.e. searching comments as well) to me.

@bmeneg
Copy link
Copy Markdown
Collaborator

bmeneg commented Jan 14, 2021

do you think it would be useful a "--full-match" flag for adding double-quotes in the terms and forcing that "exact" (it still is case insensitive) match?
It would basically be the same as lab issue search "\"search terms\"".

Do you mean to both lab mr list and lab issue list? IMHO those two commands should be as consistent as possible.

Yes! Sorry. It must be for both :)

That said, a flag does sound more convenient than escaping quotes on the command line. I'd prefer --exact-match over --full-match though - the latter suggests something like full-text search (i.e. searching comments as well) to me.

Hmm, although I agree with your concern with --full-match, I also think --exact-match is somewhat misleading, since it gives the impression of a case-sensitive search, which is not. What about only --match ?

@fmuellner
Copy link
Copy Markdown
Contributor Author

What about only --match ?

Maybe as a string flag (lab issue list --match="foo bar" instead of lab issue list "foo bar" - but then what about lab issue list --match="foo bar" baz?). For a boolean, the name sounds too generic to me.

I also think --exact-match is somewhat misleading, since it gives the impression of a case-sensitive search

Right, that's a concern.

OTOH, both Google and DuckDuckGo use "exact match/term":
https://support.google.com/websearch/answer/2466433?hl=en
https://help.duckduckgo.com/duckduckgo-help-pages/results/syntax/

And in both cases, the search is case insensitive (in fact, searching for "cats and dogs" turns up "Cats & Dogs" as top result in both for me)

@bmeneg
Copy link
Copy Markdown
Collaborator

bmeneg commented Jan 14, 2021

What about only --match ?

Maybe as a string flag (lab issue list --match="foo bar" instead of lab issue list "foo bar" - but then what about lab issue list --match="foo bar" baz?). For a boolean, the name sounds too generic to me.

Yeah.. using it as a string would be confusing.

I also think --exact-match is somewhat misleading, since it gives the impression of a case-sensitive search

Right, that's a concern.

OTOH, both Google and DuckDuckGo use "exact match/term":
https://support.google.com/websearch/answer/2466433?hl=en
https://help.duckduckgo.com/duckduckgo-help-pages/results/syntax/

And in both cases, the search is case insensitive (in fact, searching for "cats and dogs" turns up "Cats & Dogs" as top result in both for me)

Hmmm. let's use --exact-match then. However, if possible, let it clear that the search is case-insensitive in the flag description.

Thanks @fmuellner !

@prarit
Copy link
Copy Markdown
Collaborator

prarit commented Jan 21, 2021

Hi, due to no fault of yours the lab test code was broken. I have fixed the tests in #575 and #573 . Can you please rebase to see if your build will past testing? My sincere apologies for the inconvenience.

Gitlab support matching on the exact search terms by enclosing the
search strings in double quotes (as is common for web search engines
as well).

This requires careful quoting or escaping on the command line - for
example '"foo bar"' or "\"foo bar\"" - which is cumbersome; make
this more convenient by adding a dedicated command line flag that
takes care of the quoting.
Search is as useful for merge requests as for issues, so support
the same option as `lab issue list`.
Copy link
Copy Markdown
Collaborator

@bmeneg bmeneg left a comment

Choose a reason for hiding this comment

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

LGTM

@bmeneg bmeneg merged commit e67df2b into zaquestion:master Jan 22, 2021
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.

3 participants