Too much whitespace in new-app description

[smarterclayton]

This wasn't happening earlier in the release.

$ oc new-app --template=kube-system/heapster-standalone
--> Deploying template "kube-system/heapster-standalone" for "kube-system/heapster-standalone" to project myproject

     Heapster Metrics (Standalone)
     ---------
     A simple metrics solution for an OpenShift cluster. Expects to be installed in the 'kube-system' namespace.



     * With parameters:
        * MASTER_URL=https://kubernetes.default.svc
        * IMAGE_PREFIX=openshift/origin-
        * IMAGE_VERSION=latest
        * METRIC_RESOLUTION=15s
        * STARTUP_TIMEOUT=500
        * NAMESPACE=kube-system

[bparees]

you sure at least some of those newlines aren't in the template description? I only see 2:

$ oc new-app -f application-template-stibuild.json 
--> Deploying template "brokersdk/ruby-helloworld-sample" for "application-template-stibuild.json" to project brokersdk

     ruby-helloworld-sample
     ---------
     This example shows how to create a simple ruby application in openshift origin v3


     * With parameters:

[smarterclayton]

This looks wrong: #13380

[bparees]

This looks wrong: #13380

@smarterclayton is that what you intended to link? it seems unrelated to this issue so i don't know if you're saying that PR (retrying pull image actions) is also wrong or you linked the wrong thing.

[bparees]

the only recent change that might affect the output is this:
https://github.com/openshift/origin/blame/master/pkg/generate/app/cmd/template.go#L44

perhaps the decode is injecting an empty message field in the result object that results in extra newlines being printed out.

[smarterclayton]

An empty message field should be ignored I think (if it's only whitespace).

…

On Tue, Mar 21, 2017 at 10:11 PM, Ben Parees ***@***.***> wrote: the only recent change that might affect the output is this: https://github.com/openshift/origin/blame/master/pkg/ generate/app/cmd/template.go#L44 perhaps the decode is injecting an empty message field in the result object that results in extra newlines being printed out. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13492 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p2PlCZPS2r6Z_upsfmLsRFoRDd3tks5roINHgaJpZM4MkZ9U> .

[bparees]

@smarterclayton yeah should be, the current code doesn't do that (just checks for len(0))

[gabemontero]

There is a new line in the text being returned by description := result.Annotations["description"] at https://github.com/openshift/origin/blob/master/pkg/generate/app/cmd/template.go#L72

I confirmed by adding a temporary glog.V(0).Infof("Descr is %s additional text", description) ... the "additional text" was on a different line

Additional lines are inserted by https://github.com/openshift/origin/blob/master/pkg/generate/app/cmd/template.go#L80 and https://github.com/openshift/origin/blob/master/pkg/generate/app/cmd/template.go#L86 in this particular example.

There were no injections from the decode stuff that I could find.

Removing https://github.com/openshift/origin/blob/master/pkg/generate/app/cmd/template.go#L86 seems like a no brainer to me. It does look less awkward with just that change.

Do we want to chase down the new line in the annotation? In theory, we could make some of the fmt.Fprintln(out) conditional based on some analysis of the \n in the strings printed ... but perhaps that is excessive.

[bparees]

There were no injections from the decode stuff that I could find.

yeah in hindsight the decode is only operating on the objects, not the template fields themselves. but i see no other reason for this to have changed.

I do think it makes sense to just print the newline if the string itself does not end in a newline.

but i'm still not seeing how clayton's example gets to 3 blank lines:

somedescription\n (field value includes a newline)
blank (from fmt.Println after printing the description)
blank (from fmt.Println after printing message+description)
blank (from ??? possibly from printing an empty message?)

• with parameters blah blah

your proposal would get us down to 2 blanks, but we really should be at one.

(assuming i'm reading all this correctly)

[gabemontero]

On Fri, Mar 24, 2017 at 3:49 PM, Ben Parees ***@***.***> wrote: There were no injections from the decode stuff that I could find. yeah in hindsight the decode is only operating on the objects, not the template fields themselves. but i see no other reason for this to have changed. I do think it makes sense to just print the newline if the string itself does not end in a newline. but i'm still not seeing how clayton's example gets to 3 blank lines: somedescription\n (field value includes a newline) blank (from fmt.Println after printing the description) blank (from fmt.Println after printing message+description) blank (from ??? possibly from printing an empty message?)

There was no blank from an empty message ... confirmed via trace we did not go down that path. I think the `formatString` function is adding something as well, but have not confirmed that yet. That path changed earlier this year (the commit was created in late Jan, but of course it could have merged later). But I wanted some feedback before going too far down one path vs. another. If I read you right, you are amenable to analyzing the description, etc. for new-lines and adjusting newlines this code path inserts accordingly. I'll start down hat path.

…

- with parameters blah blah your proposal would get us down to 2 blanks, but we really should be at one. (assuming i'm reading all this correctly) — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#13492 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADbadBi48QmT2LW5TFowVCP4_ZGjAKjYks5rpB5LgaJpZM4MkZ9U> .

[gabemontero]

Yep formatString is also adding additional lines. Hope to have something to review before EOB today.

[gabemontero]

Also, upon further review, I think formatString is trying to account for whether the string ends with a new line.

Between the fix I have there, plus the removal on one unneeded fmt.Fprintf(out) we should be good.