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

Implement conditional constants #1373

Merged
merged 2 commits into from
Aug 10, 2024
Merged

Implement conditional constants #1373

merged 2 commits into from
Aug 10, 2024

Conversation

Pablete1234
Copy link
Member

@Pablete1234 Pablete1234 commented Jul 28, 2024

Implements section 3.2.1 in #1181, in the second bulletpoint way, where constants are processed at the same time as includes. This means that constants can be used in conditionals (<if> / <unless>), but only if defined before the conditional.

TLDR;

  • Constants now have a fallback attribute, defaults to false. If true, the constant is a fallback (a default) and will NOT override a prior declaration.
    • Consider making all your includes use fallback="true", as you probably do not want to ever override what the map sets
    • You can declare it in the parent <constants> tag and it applies to all children
  • Conditionals (<if> and <unless>) can now work on constants, as long as they were defined before the conditional.
  • This is only really useful for includes, in a single-map xml this is pretty useless.
  • Also adds min-server-version and max-server-version attributes to conditionals

Examples

the-include.xml:

<map>
  <actions>
    <if constant="compass-time">
      <trigger action="compass-kit">
          <filter><time>${compass-time}</time></filter>
      </trigger>  
    </if>
  </actions>
</map>

Map A.xml:

<map>
  <constant id="compass-time">5m</constant>
  <include id="the-include"/>
</map>

Map B:

<map>
  <include id="the-include"/>
</map>

Map C:

<map>
  <constant id="compass-time">5m</constant>
  <include id="the-include"/>
  <constant id="compass-time" delete="true"/>
</map>

This is the most basic usage of the constant in a conditional, this defaults to if anyone has set a value for the constant or not.

  • Map A: it would configure the trigger, as the constant was declared
  • Map B: it wouldn't configure the trigger, as constant wasn't declared
  • Map C: it would configure the trigger as it's "5m" when include and its conditional is checked. However would fail later as the filter would become: <filter> </filter> when time is deleted.

IMPORTANT: constants in conditionals only work if declared BEFORE the conditional is evaluated, and CAN be different values.

Constant fallback attribute

Because of this behavior where defining the constant BEFORE the include is needed, but you likely want your definition to prevail over the includes', it makes sense for all includes to declare their constants as fallback:

<map>
  <constants fallback="true">
    <constant id="compass-kit">5m</constant>
  </constants>
  <actions>
    <if constant="compass-time" constant-comparison="defined value"> <!-- more on this later -->
      <trigger action="compass-kit">
          <filter><time>${compass-time}</time></filter>
      </trigger>  
    </if>
  </actions>
</map>

Now these constants can be overwritten before the include (so that they work on conditionals too) without the include imposing them. So all maps using the include by default use 5m, and maps who want to disable it can just set it to delete="true". Note that fallback can be specified either on the constant or the constants parents, similar to many other features in PGM.

All conditional constant options

Now, can you check more than just the conditional being set? the answer is yes, but it gets a bit complicated:

Attributes:
constant: (required) the id of the constant to check against
constant-value: (depends on compare) the value to check for, this depends on the type of comparison done
constant-comparison: (optional) the type of comparison, like just checking if it is defined, or checking if it matches specific values. If not set, defaults to defined_value if no value, or equals if there's a value.

(Note: the default changed from defined to defined_value in #1381, this comment was updated to reflect it)

All possible constant-comparison possibilities (case-insensitive):

  • UNDEFINED: checks that the constant has not been defined to anything
    • eg: <if constant="const" constant-comparison="undefined"/>
  • DEFINED: checks the constant has been defined, to anything (either to delete, or to a value)
    • eg: <if constant="const" constant-comparison="defined"/>
  • DEFINED_DELETE: the constant has been defined as a delete
    • eg: <if constant="const" constant-comparison="defined delete"/>
  • DEFINED_VALUE: the constant has defined as an actual value (not a delete)
    • eg: <if constant="const" constant-comparison="defined value"/>
    • Note: this is the default if constant-value attribute is not defined
  • EQUALS: the constant equals to the attribute constant-value
    • eg: <if constant="const" constant-value="5m" />
    • Note: this is the default if constant-value is defined
  • CONTAINS: the constant is one of the the comma-separated list of values in constant-value
    • eg: <if constant="const" constant-value="a,b,c" constant-comparison="contains"/>
  • REGEX: checks if the constant matches the regex
    • eg: <if constant="const" constant-value="[0-9]+" constant-comparison="regex"/>
  • RANGE: check if the constant, interpreted as a number, is in the range
    • eg: <if constant="const" constant-value="0..12" constant-comparison="range"/>
    • checks if const is a number between 0 and 12
    • Note: if the constant is not numeric this will be an XML error.

Last notes

I hope this is enough to understand all the possibilities here, good luck to the documenters.

@Pablete1234 Pablete1234 added feature New feature or request discussion Just talking needs testing This PR requires further testing labels Aug 4, 2024
@Pablete1234 Pablete1234 marked this pull request as ready for review August 10, 2024 01:55
cswhite2000
cswhite2000 previously approved these changes Aug 10, 2024
Signed-off-by: Pablo Herrera <[email protected]>
@Pablete1234
Copy link
Member Author

While i think this may not have gotten enough testing, basic testing was done and if bugs surface we can fix them at a later time. The testing in this case doesn't need to hold back the impl being more widely available.

@mitchts
Copy link

mitchts commented Aug 12, 2024

The fallback="true" attribute only appears to work when defined on the constant itself rather than on the parent <constants/>.

The following will always use Fallback as the value even if the map including the file defines const.

<constants fallback="true">
    <constant id="const">Fallback</constant>
</constants>

However the below works with const only being Fallback when the including map does not already define it.

<constants>
    <constant id="const" fallback="true">Fallback</constant>
</constants>

Is this intentional? As the example provided is different to the actual behaviour.

@Pablete1234
Copy link
Member Author

not intentional, will take a look and fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Just talking feature New feature or request needs testing This PR requires further testing
Development

Successfully merging this pull request may close these issues.

3 participants