groovy

Multiple MarkupBuilder Issues

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.6.4, 1.7-beta-1
  • Fix Version/s: 1.7-beta-2, 1.6.6
  • Component/s: None
  • Labels:
    None
  • Environment:
    win 7 64, java 1.6.0_16/64
  • Number of attachments :
    0

Description

I have this simple test script:

import groovy.xml.*

def out = new StringWriter()
def mkp = new MarkupBuilder(out)
mkp.table(id: 'psds', border: 0) {
  tbody {
    tr {
      td(id: 999) { mkp.yield 999 }
      td(id: 99) { mkp.yieldUnescaped 99 }
      td(id: 88) { mkp.yieldUnescaped 'hello' }
      td(id: 77) { mkp.yieldUnescaped "hello2" }
      td(id: 66) { mkp.yieldUnescaped """hello3""" }
      // gives compilation error "unexpected token: } at line: 12, column: 52": td(id: 55) { mkp.yieldUnescaped /hello4/ }
    }
  }
}
    
println out.toString()

This gives:

<table id='psds' border='0'>
  <tbody>
    <tr>
      <td id='999'>
        <yield>999</yield>
      </td>
      <td id='99'>
        <yieldUnescaped>99</yieldUnescaped>
      </td>
      <td id='88'>hello</td>
      <td id='77'>hello2</td>
      <td id='66'>hello3</td>
    </tr>
  </tbody>
</table>

IMHO: there are a couple of issues here...

  • The behaviour of yield/yieldUnescaped with non-strings
    The methods should not surround the values with <yield>...</yield>, etc.
    With HTML this is a nuisance, but if one were trying to validate against an XML schema, this would be disastrous.
  • The compilation error with slashy strings

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Paul King added a comment -

Just a comment that the slashy string part wasn't addressed. I believe that this should be a separate issue anyway and certainly isn't related at all to builders - it is a generic issue with placement of slashy strings in Groovy.

Also, the fix raises another concern around documentation. At one point, due to the semantics originally used for closures within builders, I believe that the 'mkp' property was compulsory; now this is not the case. We should document the fact that both are now supported. But more importantly, we need to document the escape mechanism when you want an element called 'yield' for instance. Someone wanting numbers inside a yield element for a scientific experiment may now be surprised by the new behavior. I believe the change is desired but we need to document the sanctioned escape mechanism.

Show
Paul King added a comment - Just a comment that the slashy string part wasn't addressed. I believe that this should be a separate issue anyway and certainly isn't related at all to builders - it is a generic issue with placement of slashy strings in Groovy. Also, the fix raises another concern around documentation. At one point, due to the semantics originally used for closures within builders, I believe that the 'mkp' property was compulsory; now this is not the case. We should document the fact that both are now supported. But more importantly, we need to document the escape mechanism when you want an element called 'yield' for instance. Someone wanting numbers inside a yield element for a scientific experiment may now be surprised by the new behavior. I believe the change is desired but we need to document the sanctioned escape mechanism.
Hide
Permalink
alpheratz added a comment -

By playing around a bit I have JUST discovered another 'shortcut', as shown in the first two 'td's:

import groovy.xml.*

def out = new StringWriter()
def mkp = new MarkupBuilder(out)
def player = new Expando()
player.name = "Dierk"
player.greeting = { "Hello, my name is $name" }
player.toString = { greeting() }
mkp.table(id: 'psds', border: 0) {
  tbody {
    tr {
      td(id: 'player', player)
      td(id: 97, 97)
      td(id: 88) { mkp.yieldUnescaped 'hello' }
      td(id: 77) { mkp.yieldUnescaped "hello2" }
      td(id: 66) { mkp.yieldUnescaped """hello3""" }
      // gives compilation error "unexpected token: } at line: 12, column: 52": td(id: 55) { mkp.yieldUnescaped /hello4/ }
    }
  }
}
    
println out.toString()

This makes for MUCH nicer code than for the other td's. Worthy of explicit documentation. I was quite tired of putting all those "mkp.yieldUnescaped" calls in place...I previously thought that if one specified an attribute map for the td, one HAD to give a closure (possibly my immaturity with the language, but still...).

Show
alpheratz added a comment - By playing around a bit I have JUST discovered another 'shortcut', as shown in the first two 'td's: 
import groovy.xml.*

def out = new StringWriter()
def mkp = new MarkupBuilder(out)
def player = new Expando()
player.name = "Dierk"
player.greeting = { "Hello, my name is $name" }
player.toString = { greeting() }
mkp.table(id: 'psds', border: 0) {
  tbody {
    tr {
      td(id: 'player', player)
      td(id: 97, 97)
      td(id: 88) { mkp.yieldUnescaped 'hello' }
      td(id: 77) { mkp.yieldUnescaped "hello2" }
      td(id: 66) { mkp.yieldUnescaped """hello3""" }
      // gives compilation error "unexpected token: } at line: 12, column: 52": td(id: 55) { mkp.yieldUnescaped /hello4/ }
    }
  }
}
    
println out.toString()
This makes for MUCH nicer code than for the other td's. Worthy of explicit documentation. I was quite tired of putting all those "mkp.yieldUnescaped" calls in place...I previously thought that if one specified an attribute map for the td, one HAD to give a closure (possibly my immaturity with the language, but still...).
Hide
Permalink
alpheratz added a comment -

Well strike me dead!

Just checked the javadoc: http://groovy.codehaus.org/api/groovy/xml/MarkupBuilder.html

It shows: 

...
c( a2:'two', 'blah' )
...

Apologies.

Show
alpheratz added a comment - Well strike me dead! Just checked the javadoc: http://groovy.codehaus.org/api/groovy/xml/MarkupBuilder.html It shows: 
...
c( a2:'two', 'blah' )
...
Apologies.
Hide
Permalink
Paul King added a comment -

Mostly for the sake of future google requests that land here ... Yes, having both a value and attributes makes for compact builder code. That style isn't recommended though if you also have other nested content (mixed markup) as there is no way to specify the ordering between the implicit nested content (the value) and the explicit nested content. StreamingMarkupBuilder goes so far as to forbid it. MarkupBuilder just places the value first.

Show
Paul King added a comment - Mostly for the sake of future google requests that land here ... Yes, having both a value and attributes makes for compact builder code. That style isn't recommended though if you also have other nested content (mixed markup) as there is no way to specify the ordering between the implicit nested content (the value) and the explicit nested content. StreamingMarkupBuilder goes so far as to forbid it. MarkupBuilder just places the value first.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.