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

Add `truncate` function for Go templates #30484

Merged
merged 1 commit into from Jan 31, 2017

Conversation

@yongtang
Member

yongtang commented Jan 26, 2017

This fix is part of the discussion in #28199 about using truncate to replace --no-trunc.

As part of the fix, a new function truncate has been added for Go templates so that it is possible to use

docker stack services --format "{{truncate .ID 5}}: {{.Mode}} {{.Replicas}}"

to show truncated ID (limited by length).

A unit test has been added.

This fix is related to #28199.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Jan 27, 2017

Member

LGTM

Member

dnephin commented Jan 27, 2017

LGTM

@dnephin

The tests could use the assertion helpers. I think it makes them a lot easier to read when they are shorter.

The error messages are also consistent that way, and include things like "Expected no error, but got: ..."

Show outdated Hide outdated pkg/templates/templates_test.go Outdated
Show outdated Hide outdated pkg/templates/templates_test.go Outdated
Show outdated Hide outdated pkg/templates/templates_test.go Outdated
Show outdated Hide outdated pkg/templates/templates.go Outdated
@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jan 27, 2017

Member

Thanks @dnephin the PR has been updated with unit tests using assertion helpers. Other tests in pkg/templates/templates_test.go has also been updated to use it. (not necessarily related to shorten but the change is small enough).

Member

yongtang commented Jan 27, 2017

Thanks @dnephin the PR has been updated with unit tests using assertion helpers. Other tests in pkg/templates/templates_test.go has also been updated to use it. (not necessarily related to shorten but the change is small enough).

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jan 27, 2017

Member

Thanks @cpuguy83 @vdemeester for the comment. Now shorten takes the int for the max length of the truncation. Please take a look.

Member

yongtang commented Jan 27, 2017

Thanks @cpuguy83 @vdemeester for the comment. Now shorten takes the int for the max length of the truncation. Please take a look.

@cpuguy83

LGTM

@dnephin

LGTM

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Jan 27, 2017

Contributor

The correct word your looking for is truncate.

Contributor

stevvooe commented Jan 27, 2017

The correct word your looking for is truncate.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jan 28, 2017

Member

I agree that truncate would be better; it's also consistent with the current --no-trunc flag, so easier to explain that they relate.

Moving it back to design for a bit

Member

thaJeztah commented Jan 28, 2017

I agree that truncate would be better; it's also consistent with the current --no-trunc flag, so easier to explain that they relate.

Moving it back to design for a bit

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jan 28, 2017

Member

We currently have

	"json":
	"split"
	"join"
	"title"
	"lower"
	"upper"
	"pad"

I am thinking maybe we could consider trunc?
It seems truncate is a litter long compared with other functions. Though either is fine for me.

Member

yongtang commented Jan 28, 2017

We currently have

	"json":
	"split"
	"join"
	"title"
	"lower"
	"upper"
	"pad"

I am thinking maybe we could consider trunc?
It seems truncate is a litter long compared with other functions. Though either is fine for me.

@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Jan 30, 2017

Collaborator

👍 for truncate.

Collaborator

tiborvass commented Jan 30, 2017

👍 for truncate.

Add `truncate` function for Go templates
This fix is part of the discussion in 28199 about using
`truncate` to replace `--no-trunc`.

As part of the fix, a new function `truncate` has been
added for Go templates so that it is possible to use
```
docker stack services --format "{{truncate .ID 5}}: {{.Mode}} {{.Replicas}}"
```
to show truncated ID.

A unit test has been added.

This fix is related to 28199.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

@yongtang yongtang changed the title from Add `shorten` function for Go templates to Add `truncate` function for Go templates Jan 30, 2017

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jan 30, 2017

Member

Thanks all for the review. The PR has been updated with truncate. The title and description of the PR has also been updated. Please take a look.

Member

yongtang commented Jan 30, 2017

Thanks all for the review. The PR has been updated with truncate. The title and description of the PR has also been updated. Please take a look.

@LK4D4

LK4D4 approved these changes Jan 31, 2017

LGTM

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Jan 31, 2017

Contributor
Contributor

LK4D4 commented Jan 31, 2017

@vdemeester

LGTM 🎉

@vdemeester vdemeester merged commit 818ef47 into moby:master Jan 31, 2017

4 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 30123 has succeeded
Details
janky Jenkins build Docker-PRs 38736 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 9764 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Jan 31, 2017

@yongtang yongtang deleted the yongtang:28199-shorten branch Jan 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment