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

Add on directive to switch as an alternative to case #106

Open
wants to merge 3 commits into
base: 2.3-gae
Choose a base branch
from

Conversation

scrhartley
Copy link

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
Copy link

jido commented Feb 13, 2024

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

{
// A Switch with Case supports break, but not one with On.
// Do it this way to ensure backwards compatibility.
breakableDirectiveNesting--;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will that also work when switch-case is nested inside switch-on?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few lines up, at the SwitchBlock level, there's an increment. As soon as we notice we're in the on case, then we undo the increment. If there was a nested switch-case then it would be incremented again. This counter is only used at parse time to determine if we're in a context where break is allowed. Because on doesn't allow a closing tag, I don't think there's a possibility for a disallowed break to accidentally be allowed between on tags, although the parser probably wouldn't allow that anyway.

@scrhartley
Copy link
Author

scrhartley commented Feb 13, 2024

@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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants