code review 105070043: go.tools/present: allow intentionally empty parameters

go.tools/present: allow intentionally empty parameters

Fixes issue 7613.

[kortschak, 2014-06-11 06:51:54]

Hello adg@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.tools

[adg, 2014-06-12 00:26:01]

Looks good to me. Rob?

https://codereview.appspot.com/105070043/diff/60001/present/doc.go
File present/doc.go (right):

https://codereview.appspot.com/105070043/diff/60001/present/doc.go#newcode173
present/doc.go:173: allows preserving the aspect ratio of the image when
scaling.
s/allows preserving/preserves/

[r, 2014-06-12 00:31:01]

Where is the facility used?

[kortschak, 2014-06-12 00:53:46]

PTAL

It can currently be worked around by saying `.image foo.png 200 0` for example,
but this clarifies that the empty argument is there as a place holder, rather
than a real zero. The change here is for human readability.

FWIW I use this approach above in my lectures which all depend on maintaining
aspect ratios for scientific figures.

On Thu, 2014-06-12 at 10:40 +1000, Andrew Gerrand wrote:
> There have been several occasions where I've had to compute the appropriate
> dimensions. If this feature were available I would have just specified the
> horizontal or vertical and let the computer do the computation for me.
> 
> 
> On 12 June 2014 10:31, <r@golang.org> wrote:
> 
> > Where is the facility used?

https://codereview.appspot.com/105070043/diff/60001/present/doc.go
File present/doc.go (right):

https://codereview.appspot.com/105070043/diff/60001/present/doc.go#newcode173
present/doc.go:173: allows preserving the aspect ratio of the image when
scaling.
On 2014/06/12 00:26:00, adg wrote:
> s/allows preserving/preserves/

Done.

[r, 2014-06-12 16:04:19]

that's pretty magical. no wonder i didn't see how it worked. how about a comment
in image.go explaining what's going on?

[kortschak, 2014-06-12 22:36:14]

PTAL

On 2014/06/12 16:04:19, r wrote:
> that's pretty magical. no wonder i didn't see how it worked. how about a
comment
> in image.go explaining what's going on?

[r, 2014-06-12 22:44:01]

LGTM

[r, 2014-06-12 22:44:58]

*** Submitted as
https://code.google.com/p/go/source/detail?r=1e6e75afb0cd&repo=tools ***

go.tools/present: allow intentionally empty parameters

Fixes issue 7613.

LGTM=r
R=adg, r
CC=golang-codereviews
https://codereview.appspot.com/105070043

Committer: Rob Pike <r@golang.org>

[kortschak, 2014-06-12 23:03:39]

Late thought.

This does not check that the argument is "_", just that it matches /_.*/. If
either of you think this is important I will send another CL to fix that.

On 2014/06/12 22:44:58, r wrote:
> *** Submitted as
> https://code.google.com/p/go/source/detail?r=1e6e75afb0cd&repo=tools ***
> 
> go.tools/present: allow intentionally empty parameters
> 
> Fixes issue 7613.
> 
> LGTM=r
> R=adg, r
> CC=golang-codereviews
> https://codereview.appspot.com/105070043
> 
> Committer: Rob Pike <mailto:r@golang.org>