Skip to content

Fix Nested If Tags#341

Open
28Smiles wants to merge 15 commits intomichael-milette:masterfrom
28Smiles:multilevel-bug
Open

Fix Nested If Tags#341
28Smiles wants to merge 15 commits intomichael-milette:masterfrom
28Smiles:multilevel-bug

Conversation

@28Smiles
Copy link
Contributor

@28Smiles 28Smiles commented May 28, 2025

{ifingroup a}{ifingroup b}Hello World{/ifingroup}{/ifingroup} -> {/ifingroup}

and so on lead to mismatches and wrong output. In stage one i added some tests for bug reproduction. Only nestings of the same filtercode category are affected.

I propose matching all nestings of groupings generating replacements for all tags at once, e.g.:

  1. Check grouping openings -> {ifingroup a}{ifingroup b}
  2. Generate Matcher: '/{ifingroup\s+a\}(.*)/{ifingroup\s+b\}(.*)\{\/ifingroup\}(.*)\{\/ifingroup\}/isuU'
  3. Replace either with:
  • 'a = true, b = true' => '$1$2$3'
  • 'a = true, b = false' => '$1$3'
  • 'a = false, b = true' => ''
  • 'a = false, b = false' => ''

I will now implement a proof of concept

@28Smiles
Copy link
Contributor Author

@michael-milette I implemented it for ifingrouping, ifnotingrouping, ifingroup, ifnotingroup for now, but the new function is pretty much plug and play for the other if-tags

@28Smiles 28Smiles changed the title Fix Nested If Statements Fix Nested If Tags May 28, 2025
@28Smiles
Copy link
Contributor Author

28Smiles commented Jun 4, 2025

Implemented for all relevant if tags known to me

@michael-milette michael-milette requested a review from Copilot June 8, 2025 19:33
@michael-milette michael-milette self-assigned this Jun 8, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses the mismatches in nested if-tags by updating the tag matching logic and adding extensive tests for ifingroup, ifingrouping, and related tags. Key changes include:

  • Removal of an unused import for clarity.
  • New tests for various nested conditions including partial tag scenarios, group membership, and profile checks.
  • Expanded coverage of ifnotingroup/ifnotingrouping and ifactivitycompleted tags within the test suite.
Comments suppressed due to low confidence (1)

tests/filter_test.php:28

  • The removal of the unused import statement 'use filter_filtercodes;' improves code clarity. Confirm that this change does not affect any indirect dependencies in the test suite.
-use filter_filtercodes;

@28Smiles
Copy link
Contributor Author

28Smiles commented Aug 7, 2025

@michael-milette Any updates?

@michael-milette
Copy link
Owner

Hi @28Smiles ,

I am very interested in your PR. I think it would be an excellent addition to FilterCodes. However, due to the extensive changes and my busy work schedule, I haven't been able to dedicate the time necessary to review the changes before integrating them. Backwards compatibility is very important here.

Out of curiosity, have you been using your patched version of FilterCodes? If so, did you experience any backwards compatibility issues where you might have had to go change existing usage of FilterCodes tags?

Best regards,

Michael

@28Smiles
Copy link
Contributor Author

28Smiles commented Aug 7, 2025

We didnt adopt it yet, since we've been waiting for the merge, but we might start using it. I tried to be as compatible as possible, even wrinting some tests for regressions and basic functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants