Skip to content
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

[JENKINS-27505] Preserve an empty line at the beginning of a textarea #1612

Conversation

@ikedam
Copy link
Member

commented Mar 21, 2015

JENKINS-27505
A blank line in the beginning of a textarea is removed.
It is caused for the specification of HTML:
http://www.w3.org/TR/html5/syntax.html#syntax-newlines

A single newline may be placed immediately after the start tag of pre and textarea elements. If the element's contents are intended to start with a newline, two consecutive newlines thus need to be included by the author.

(I could not found the description in HTML4.01)

I think my change gets too complicated as I don't know the best way to handle \r and \n in jelly files
(jelly doesn't seem to handle backslashes).
I want help of an expert of jelly.

ikedam added 2 commits Mar 19, 2015
…ne at the beginning will be removed in a textarea.
@ikedam

This comment has been minimized.

Copy link
Member Author

commented Mar 21, 2015

This result demonstrates what happens:
https://jenkins.ci.cloudbees.com/job/core/job/jenkins-core/2256/testReport/lib.form/TextAreaTest/testTextBeginningWithEmptyLine/

org.junit.ComparisonFailure: expected:<[
]begin

with
empty
li...> but was:<[]begin

with
empty
li...>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.junit.Assert$assertEquals.callStatic(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallStatic(CallSiteArray.java:50)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:157)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:169)
    at lib.form.TextAreaTest.testTextBeginningWithEmptyLine(TextAreaTest.groovy:94)

I'll commit the fix.

@@ -89,6 +89,7 @@ THE SOFTWARE.
readonly="${attrs.readonly}"
codemirror-mode="${attrs['codemirror-mode']}"
codemirror-config="${attrs['codemirror-config']}">
<j:if test="${value != null &amp;&amp; !empty(value.toString()) &amp;&amp; (value.toString().codePointAt(0) == 10 || value.toString().codePointAt(0) == 13)}"><j:whitespace>&#10;</j:whitespace></j:if>

This comment has been minimized.

Copy link
@ikedam

ikedam Mar 21, 2015

Author Member

Jelly doesn't accept backslashes (like \n)...

This comment has been minimized.

Copy link
@KostyaSha

KostyaSha Mar 26, 2015

Member

jelly and groovy has ability to unescape text, feel free to try.

This comment has been minimized.

Copy link
@ikedam

ikedam Mar 26, 2015

Author Member

I tried this in jelly

:\r\n
:${'\r\n'}
:${&quot;\r\n&quot;}
:<st:out value="\r\n" />
:<st:out value="${'\r\n'}" />
:<st:out value="${&quot;\r\n&quot;}" />

and that resulted like this:

:\r\n
:\r\n
:\r\n
:\r\n
:\r\n
:\r\n

backslashes doesn't seem handled.

Maybe I need another way to handle control characters.
Would you show me some examples to have jelly unescape texts?
I could not find good documentations for jelly...

This comment has been minimized.

Copy link
@jglick

jglick Mar 30, 2015

Member

Documentation for Jelly is weak. Generally speaking if you are trying to do anything subtle you have to switch to Groovy to get more control.

p.buildersList.add(new TextareaTestBuilder(TEXT_TO_TEST));
assertEquals(TEXT_TO_TEST, p.getBuildersList().get(TextareaTestBuilder.class).getText());
j.configRoundtrip(p);
assertEquals(TEXT_TO_TEST, p.getBuildersList().get(TextareaTestBuilder.class).getText());

This comment has been minimized.

Copy link
@jglick

jglick Mar 26, 2015

Member

Can use assertEqualDataBoundBeans to make it a bit more concise if you like.

@Issue("JENKINS-27505")
@Test
public void testTextBeginningWithEmptyLine() {
def TEXT_TO_TEST = "\nbegin\n\nwith\nempty\nline\n\n";

This comment has been minimized.

Copy link
@jglick

jglick Mar 26, 2015

Member

What about two initial newlines?

This comment has been minimized.

Copy link
@jglick

jglick Mar 30, 2015

Member

(resolved)

@jglick

This comment has been minimized.

Copy link
Member

commented Mar 26, 2015

👍

@ikedam

This comment has been minimized.

Copy link
Member Author

commented Mar 30, 2015

Updated tests:

  • Added a test for two initial new lines.
  • Now uses assertEqualDataBoundBeans. That's really useful.
}

@Issue("JENKINS-27505")
@Test

This comment has been minimized.

Copy link
@jglick

jglick Mar 30, 2015

Member

BTW it would save test execution time to collapse these three test cases.

@jglick

This comment has been minimized.

Copy link
Member

commented Mar 30, 2015

👍

@ikedam

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2015

Collapsed test cases.

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Oct 6, 2015

👍

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Nov 15, 2015

I'm merging this implementation, because it has got more votes. #1643 may be considered as a future improvement

oleg-nenashev added a commit that referenced this pull request Nov 15, 2015
…artsEmptyLine

[JENKINS-27505] Preserve an empty line at the beginning of a textarea
@oleg-nenashev oleg-nenashev merged commit b23b3cf into jenkinsci:master Nov 15, 2015
@Test
public void testText() {
T1:{
def TEXT_TO_TEST = "some\nvalue\n";

This comment has been minimized.

Copy link
@lanwen

lanwen Nov 16, 2015

Member

why not just create 3 test methods (with @ClassRule)?

@lanwen

This comment has been minimized.

Copy link
Member

commented Nov 16, 2015

@ikedam You can use @ClassRule for jenkins and separate test class. Collapsing tests is bad pattern with produces test smell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.