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

Separator not emitted for nulls until a value is hit #303

Open
rrs opened this issue Aug 24, 2022 · 4 comments
Open

Separator not emitted for nulls until a value is hit #303

rrs opened this issue Aug 24, 2022 · 4 comments
Labels

Comments

@rrs
Copy link

rrs commented Aug 24, 2022

We use string templates for a variety of data exports. One of which is simple CSV.

If we have an input from a table of data where as long as the first value in each row is non null then the CSV is correctly formed. When the first value is a null a separator is not emitted until there is a value.

Imagine the following table of data

Name, Age, Color
Ron, 10, Pink
Paul,15,Brown

and the template

  printRow(row) ::= <%
    <items.Columns:{ k | <row.(k)>}; separator=","><\n>
  %>
  
  outputTemplate(items) ::= <%
    <items.Columns; separator=","><\n>
    <items.Rows:printRow()>
  %>

this will output
Name, Age, Color
Ron, 10, Pink
Paul,15,Brown

if however the input data was
Name, Age, Color
Ron, 10, Pink
null,15,Brown

Then the output would be

Name, Age, Color
Ron, 10, Pink
15,Brown

and not

Name, Age, Color
Ron, 10, Pink
,15,Brown

I can see it is because of this section of code

image

because the seenAValue is false until the first value in the enumerable is non null.

In this case there is a work around, which is to substitute nulls with "" which will generate for the example above

Name, Age, Color
Ron, 10, Pink
"",15,Brown

However I was looking at the possibility of adding another option like emitAll="true" or something like that which would be used along with the separator option to still output, something like

boolean needSeparator = (seenAValue || (emitAll && separator!=null)) &&
               separator!=null &&            // we have a separator and
               (iterValue!=null ||           // either we have a value
                   options[Option.NULL.ordinal()]!=null); // or no value but null option

However I do not have a clear idea of the impact this could have and was hoping someone for familiar with the code could advise me

@parrt
Copy link
Member

parrt commented Aug 25, 2022

I can't remember but I thought we had a template option that would test whether value is null or not and use a replacement. Oh, yeah, this thing: options[Option.NULL.ordinal()]!=null.

@rrs
Copy link
Author

rrs commented Aug 25, 2022

At this point its worth mentioning I am debugging the c# version so its possible there are differences. The codebase is largely neglected. However as those parts of the code are identical I thought I would ask here. Seems like the need to gobble up nulls at the start of a series is necessary to support some of the formatting allowed in the templates. Its the fact that the same code is also used with the separator option that gives me some hope it can be tweaked.

I will get a simple Java version going if I can to test it, but it seems in this context that null never gets triggered

image

image

My 'List' that is being iterated is actually an OrderedDictionary so it's possible that in a simpler context of just a plain list the null gets realised but here it doesn't. I'll have another play later now I have some more leads to follow.

Something else is that the value ultimately gets written here

image

and not in the with options variant, I don't quite understand how those opcodes are generated at this point

@rrs
Copy link
Author

rrs commented Aug 25, 2022

Here is a very simple example project in Java exhibiting the same behaviour

https://github.com/rrs/StringTemplateEmitSeparator

@parrt
Copy link
Member

parrt commented Aug 26, 2022

Dang. Does seem broken (haven't run code yet though).

@parrt parrt added the type:bug label Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants