Import the real "Run only if" condition from Tasker ConditionList - #13
Open
felalex wants to merge 1 commit into
Open
Import the real "Run only if" condition from Tasker ConditionList#13felalex wants to merge 1 commit into
felalex wants to merge 1 commit into
Conversation
Tasker exports a "Run only if" guard as a sibling <ConditionList> element
on any action (85 of 118 occurrences in a real backup were on ordinary
actions like Set Variable, not just If/Else If), not as flat <Str> args.
The importer never read it, so every guarded action -- and every If's own
test expression -- silently imported as unconditional/"true" with no
warning, on 118 of 210 actions in that backup across 8 action codes.
This parses <ConditionList><Condition><lhs>/<op>/<rhs> into this app's
condition syntax, covering the full known Tasker op set (0-9 comparisons,
12/13 is_set/not_set). Because flow.if reads its test expression from
args["condition"] specifically (both at runtime in TaskRunner.stepControl
and in the action editor's existingActionArgValue), the parsed value is
written there too, not just onto the generic action.condition guard field
-- otherwise an imported "If" editor would show its required condition
field blank, and re-exporting it would silently degrade back to "true",
despite the import having produced a working condition.
A multi-condition list (Tasker AND/OR chains) degrades to just its first
condition, and an out-of-range <op> code drops the condition entirely --
both cases are now reported via lossyWarning instead of failing silently
like the original bug.
The Matches/Doesn't Match (op 2/3) evaluator was rewritten to use this
project's existing RE2/compileLinearRegex infrastructure instead of
Kotlin's backtracking Regex, since the wildcard pattern is now built from
imported condition data at runtime rather than a fixed literal -- this
repo has shipped three prior desktop/Android ICU regex divergences per
CONTRIBUTING.md, so a new instrumented test exercises it on-device rather
than relying on the JVM unit test suite alone. The comparison-operator
scan was also tightened to require whitespace boundaries around ~/!~ so
it can't misfire inside an ordinary value or path that happens to contain
a literal tilde, and is_set/not_set are now only read as a fallback after
a real binary comparison fails to parse, so a value that legitimately
ends in the word "is_set" isn't misread as the unary form.
Verified end-to-end on a Pixel 9 Pro XL: imported a real Tasker backup,
confirmed both flow-control and ordinary-action guards evaluate and
display correctly, then rebuilt/reinstalled and reran the full suite.
Test commands run:
./gradlew :app:testDebugUnitTest
./gradlew :app:lintDebug
./gradlew :app:connectedDebugAndroidTest \
-Pandroid.testInstrumentationRunnerArguments.class=com.opentasker.core.engine.variables.VariableExpanderConditionInstrumentedTest
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Real Tasker exports encode an action's "Run only if" guard as a sibling
<ConditionList><Condition><lhs>/<op>/<rhs></Condition></ConditionList>element, not as flat<Str>args. The importer never read it. On a real Tasker backup this meant 118 of 210 imported actions across 8 action codes silently lost their guard/condition and imported as unconditional (or, forflow.ifspecifically, as the literal"true") with no warning at all. 85 of those 118 occurrences were on ordinary actions like Set Variable, not just If/Else If — so the bug wasn't scoped to flow control.What changed
TaskerXmlImport.kt: parses<ConditionList>into this app's own condition syntax, covering the full known Tasker op set (0-9comparisons,12/13is_set/not_set). Applies generically to any action type, not justflow.if.flow.ifreads its test expression fromargs["condition"]specifically — both at runtime inTaskRunner.stepControland in the action editor'sexistingActionArgValue. The parsed condition is written there too (not just onto the genericaction.conditionguard field), otherwise an imported "If" opens in the editor with its required condition field showing blank, and re-exporting it silently degrades back to"true"despite the import having produced a working condition.<op>code drops the condition entirely. Both cases now surface alossyWarninginstead of failing silently the way the original bug did.VariableExpander.kt: the Matches/Doesn't Match (op2/3) evaluator now uses this project's existing RE2/compileLinearRegexinfrastructure (with the same length guards) instead of Kotlin's backtrackingRegex, since the wildcard pattern is now built at runtime from imported condition data rather than a fixed literal. The comparison-operator scan requires whitespace boundaries around~/!~so it can't misfire inside an ordinary value or path containing a literal tilde, andis_set/not_setare only read as a fallback after a real binary comparison fails to parse, so a value legitimately ending in the wordis_setisn't misread as the unary form.Testing
Verified end-to-end on a physical device (Pixel 9 Pro XL): imported a real Tasker backup, confirmed both flow-control and ordinary-action guards evaluate correctly and the action editor displays the imported condition (not blank), then rebuilt and reinstalled the release build and reran the full suite against it.
This repo has no CI, so per CONTRIBUTING.md, the commands run locally:
All green: unit tests pass (including 8 new tests covering ConditionList parsing on both
flow.ifand ordinary actions, the is_set/not_set/Matches operators, multi-condition degradation, unmapped-op warnings, and the export round-trip), lint is clean, and all 6 instrumented tests pass on-device — added per CONTRIBUTING.md's requirement that regex changes get on-device coverage, since this repo has shipped three prior desktop-JVM/Android ICU regex divergences and the new wildcard pattern is built at runtime rather than being a fixed literal the existingproduction-regex-patterns.txtcorpus would catch.