Skip to content

Add --format to docker-search - #440

Merged
dnephin merged 1 commit into
docker:masterfrom
boaz0:search_format
Aug 22, 2017
Merged

Add --format to docker-search#440
dnephin merged 1 commit into
docker:masterfrom
boaz0:search_format

Conversation

@boaz0

@boaz0 boaz0 commented Aug 15, 2017

Copy link
Copy Markdown
Contributor

- What I did

This is a follow-up to Jeremy's PR moby/moby#31539.

This patch adds the format flag to the docker search command and by that completes moby/moby#30431.

Signed-off-by: Jeremy Chambers jeremy@thehipbot.com
Signed-off-by: Boaz Shuster ripcurld.github@gmail.com

- How I did it

  • Created cli/command/formatter/search.go and cli/command/formatter/search_test.go
  • Updated cli/command/registry/search.go and docs/reference/commandline/search.md

- How to verify it

Run unit tests

- Description for the changelog

Add format to docker search

- A picture of a cute animal (not mandatory but encouraged)

Comment thread docs/reference/commandline/search.md Outdated

### Format the output

The formatting option (`--format`) will pretty print search output

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pretty-print

Comment thread docs/reference/commandline/search.md Outdated
The formatting option (`--format`) will pretty print search output
using a Go template.

Valid placeholders for the Go template are listed below:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/are listed below:/are:

Comment thread docs/reference/commandline/search.md Outdated
| `.IsOfficial` | "OK" if image is official |
| `.IsAutomated` | "OK" if image build was automated |

When using the `--format` option, the `search` command will either

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/when using/when you use

Comment thread docs/reference/commandline/search.md Outdated
| `.IsAutomated` | "OK" if image build was automated |

When using the `--format` option, the `search` command will either
output the data exactly as the template declares or, when using the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

declares. If you use the `table directive, column headers are included as well.

Comment thread docs/reference/commandline/search.md Outdated
{% endraw %}
```

To search for images in a table format youcan use:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This example outputs a table format:

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 🐸

@codecov-io

codecov-io commented Aug 22, 2017

Copy link
Copy Markdown

Codecov Report

Merging #440 into master will decrease coverage by 0.2%.
The diff coverage is 96.36%.

@@            Coverage Diff             @@
##           master     #440      +/-   ##
==========================================
- Coverage   47.04%   46.84%   -0.21%     
==========================================
  Files         198      198              
  Lines       16349    16339      -10     
==========================================
- Hits         7692     7654      -38     
- Misses       8262     8295      +33     
+ Partials      395      390       -5

@dnephin dnephin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! LGTM

some minor comments, but nothing blocking merge

Comment thread cli/command/registry/search.go Outdated
format := options.format
if len(format) == 0 {
format = formatter.TableFormatKey
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: we could handle this default is formatter.NewSearchFormat()

}
}
return official
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor, a function could be used to remove the duplication in IsOfficial() and IsAutomated()

func (c *searchContext) formatBool(value bool) string {
    switch {
    case value && c.json:
        return "true"
    case value:
        return "[OK]"
    case c.json:
        return "false"
    }
    return ""
}

Comment thread cli/command/formatter/search_test.go Outdated
}
}

func TestSearchContext_Description(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: usually there are no underscores in test names: TestSearchContextDescription

@boaz0

boaz0 commented Aug 22, 2017

Copy link
Copy Markdown
Contributor Author

@dnephin no problemo

@dnephin

dnephin commented Aug 22, 2017

Copy link
Copy Markdown
Contributor

@mstanleyjones docs looks good now?

@mdlinville mdlinville left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved with one teensy little suggestion for improvement. Thanks! 🙌

Comment thread docs/reference/commandline/search.md Outdated

### Format the output

The formatting option (`--format`) will pretty-print search output

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/will pretty-print/pretty-prints

@boaz0

boaz0 commented Aug 22, 2017

Copy link
Copy Markdown
Contributor Author

@mstanleyjones done! thanks for your review 👍 🍡

@mdlinville

Copy link
Copy Markdown
Contributor

GMail renders that 🍡 as a radish!

Comment thread docs/reference/commandline/search.md Outdated

### Format the output

The formatting option (`--format`) will pretty-prints search output

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you forgot to take out the word "will" here.

Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Signed-off-by: Boaz Shuster <ripcurld.github@gmail.com>
@boaz0

boaz0 commented Aug 22, 2017

Copy link
Copy Markdown
Contributor Author

@mstanleyjones ☕️

@mdlinville mdlinville left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🍦

@dnephin
dnephin merged commit 05308fc into docker:master Aug 22, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.08.0 milestone Aug 22, 2017
nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
[18.03] Updating Changelog for 18.03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants