Add on directive to switch as an alternative to case

[scrhartley]

The current switch directive is not recommended due to its fall-through behavior being regarded as error-prone. To solve this problem, we introduce a new on directive as an alternative to case, which doesn't support fall-through. This new directive allows specifying multiple comma-separated conditions in order to address the primary motivator for using fall-through with case.

Example

Using case:

<#switch animal.size>
  <#case "tiny">
  <#case "small">
     This will be processed if it is small
     <#break>
  <#case "medium">
     This will be processed if it is medium
     <#break>
  <#default>
     This will be processed if it is neither
</#switch>

Using on:

<#switch animal.size>
  <#on "tiny", "small">
     This will be processed if it is small
  <#on "medium">
     This will be processed if it is medium
  <#default>
     This will be processed if it is neither
</#switch>

Details

• Mixing case and on directives is disallowed and will fail.
• If a switch contains an on, then using break or continue is not supported (applying to both on and default). In this situation, break and continue will follow the behavior of the containing scope, e.g. of a containing list. This does mean that if someone accidentally uses break with on, they could potentially become confused.

Additional Notes

• When on is not used, the legacy behavior for case/default is that continue will be treated as break. If we wish to change this, then that work can be done separately.
• If using default, the parser also allows it to appear before or between case directives, but when using on then default is only allowed after.

[jido]

In my opinion there should be a close tag </#on>. That would mainly help with whitespace management.

[scrhartley]

@jido's comment:

In my opinion there should be a close tag </#on>. That would mainly help with whitespace management.

@ddekany's mailing list response about this was:

I mean, we have the same whitespace issue with #else, and #elseif. So this is just not the feature where we address whitespace issues in general.

My comment about this: If whitespace is a concern then using break with case is still available.

[scrhartley]

@ddekany Should we be using LOOKAHEAD in the grammar for these changes (I don't know much about it)?

[ddekany]

Now that we are over the 2.3.33 release, will find time for this. Sorry for the delay. Will see if there's any issue, though if it's minor I tend to just fix it myself and merge.

[scrhartley]

Now I've had some time to sit with it, I'm rethinking break not working the same as with case. Although it's not needed, it seems like it may be less surprising if it works the same (perhaps fewer support questions). It would also provide a mechanism for people who are desperate to avoid whitespace.
Note: If we made that change, we would need to decide the behavior of continue , due to its surprising legacy handling with case.

Example trying to avoid whitespace:

<#switch animal.size>
  <#on "tiny", "small">This will be processed if it is small<#break>
  <#on "medium">This will be processed if it is medium<#break>
  <#default>This will be processed if it is neither<#break>
</#switch>

Counter-argument example instead using t (using rt seems to lead to slightly different results):

<#switch animal.size>
  <#on "tiny", "small">This will be processed if it is small<#t>
  <#on "medium">This will be processed if it is medium<#t>
  <#default>This will be processed if it is neither<#t>
</#switch>

This is more a request for opinion than me making a decision. The motivation behind this directive was to give us a switch which isn't deprecated due to fall-through, but other than that, additional complications seem unnecessary. I might have a different opinion if we weren't reusing the switch directive.

[ddekany]

<#t> expresses the intent much better than #break. I understand that the behavior of #break can cause confusion, but I guess that's something we have to live with. Or if we absolutely can't, then I prefer disallowing it inside #switch over allowing it to be used for whitespace control.

[scrhartley]

Anecdote For my own personal project where I was dealing with whitespace, I did end up with the following:

    ${' '}decoding="${decoding}"<#t>

I wanted to remove the indentation except the first space, as I had multiple attributes.

[ddekany]

See post-merge adjustments commit: https://github.com/apache/freemarker/commit/ade44352d64f3cd81a2271edb366df90ec126ce5 (I will add more Manual updates later.)

[ddekany]

Since then there were some more follow up commits, and the manual page looks like this: 2.3.24-SNASHPOT Manual / Switch. For now I assume this feature is done, but of course please find any rough edges.

[ddekany]

Ah... I just realized I messed up user lookup, and you probably don't have Contributor License Agreement. We will need that, or else this can't get into the release.

[jido]

Hi Daniel,There is a mistake in this manual page: <#case refValueN>That should be #on.Cheers— Denis. On 19 Aug 2024, at 16:45, ddekany @.***> wrote: Since then there was some more follow up commits, and the manual page looks like this: 2.3.24-SNASHPOT Manual / Switch

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[ddekany]

Thanks @jido, fixed that.

[ddekany]

@scrhartley I had to remove this from the 2.3-gae (and 2.3) branch (by hard reset... sorry), and now it's in 2.3-gae-missing-cla. Can you create a Contributor License Agreement (assuming you did not have one), and after that was confirmed by Apache, create a new PR on the same commit as earlier? Sorry for the mess... I should not have merged yet, but mixed up people and believed you had a CLA.

[ddekany]

@scrhartley Have received your ICLA - thanks! Can you create a new PR, on your last commit, so same as before? Sorry for the mess, and thanks again!