ammachado commented on code in PR #26616:
URL: https://github.com/apache/camel/pull/26616#discussion_r4055861983


##########
core/camel-core-languages/src/main/docs/modules/languages/pages/simple-operators.adoc:
##########
@@ -123,16 +123,29 @@ The syntax for the ternary operator is:
 
 [source,text]
 ----
-${leftValue} OP rightValue ? trueValue : falseValue
+${leftValue OP rightValue ? trueValue : falseValue}
 ----
 
+The whole ternary must be inside a single `${ }`. Outside `${ }` a Simple 
expression is a template,
+so a `?` and a `:` there are literal text and the operator is not evaluated. 
For example
+`${body.size()} == 0 ? ${null} : ${body[0]}` is not a ternary: it returns the 
text

Review Comment:
   Fixed in d788ecbfa6f3, by a different route than suggested.
   
   Root cause confirmed from the job log, which shows exactly one warning: 
`skipping reference to missing attribute: null`, and the doc job is 
warnings-as-errors (exit 2).
   
   The scope was narrower than the suggestion assumed: an AsciiDoc attribute 
name cannot contain `.`, `(`, `)`, `[` or `]`, so only `${null}` is an 
attribute reference. `${body.size()}` and `${body[0]}` never were. Auditing the 
file for attribute-name-shaped `{...}` outside verbatim blocks gives exactly 
one hit before the fix (`{null}`, line 131) and zero after, which matches the 
log precisely. That is also why this is the first breakage of its kind: every 
pre-existing plain span in these docs (`${date:now:HH}`, 
`${body.substring(2)}`) contains such a character, and there is no other 
`${null}` in prose anywhere in the tree.
   
   Rather than `+...+` passthrough, which appears nowhere in these language 
docs, the counter-example moved into a `[source,text]` block. Verbatim blocks 
substitute nothing, so the whole class of problem goes away, and it matches how 
every other expression on this page is already presented.
   
   _Claude Code on behalf of Adriano Machado (@ammachado)_
   
   _This was generated by an AI agent and may contain inaccuracies. Please 
verify before relying on it._



##########
catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/simple-operators.adoc:
##########
@@ -123,16 +123,29 @@ The syntax for the ternary operator is:
 
 [source,text]
 ----
-${leftValue} OP rightValue ? trueValue : falseValue
+${leftValue OP rightValue ? trueValue : falseValue}
 ----
 
+The whole ternary must be inside a single `${ }`. Outside `${ }` a Simple 
expression is a template,
+so a `?` and a `:` there are literal text and the operator is not evaluated. 
For example
+`${body.size()} == 0 ? ${null} : ${body[0]}` is not a ternary: it returns the 
text

Review Comment:
   Fixed in d788ecbfa6f3. The generated copy is a byte-for-byte mirror of the 
source page, so it carries the same `[source,text]` block and no longer 
contains an attribute-name-shaped reference outside a verbatim block. See the 
reply on the source file for why the fix is a verbatim block rather than 
`+...+` passthrough.
   
   _Claude Code on behalf of Adriano Machado (@ammachado)_
   
   _This was generated by an AI agent and may contain inaccuracies. Please 
verify before relying on it._



##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SimpleChecks.java:
##########
@@ -102,24 +102,85 @@ else if (key.startsWith("message:") && 
!key.equals("message:")) {
             // Determine predicate vs expression context
             boolean predicate = !isLogMessage && isPredicate(catalog, lines, 
i, lineIndent);
 
+            boolean parsed = true;
             try {
                 LanguageValidationResult result = predicate
                         ? catalog.validateLanguagePredicate(null, "simple", 
simpleText)
                         : catalog.validateLanguageExpression(null, "simple", 
simpleText);
                 if (!result.isSuccess()) {
                     String error = result.getShortError() != null ? 
result.getShortError() : result.getError();
                     if (error != null && !isMissingDependency(error)) {
+                        parsed = false;
                         errors.add("Line " + lineNum + ": Simple syntax error: 
" + error
                                    + aggregatedSizeHint(error, lines, i, 
lineIndent));
                     }
                 }
             } catch (Exception e) {
                 // best effort
             }
+

Review Comment:
   Renamed in d788ecbfa6f3: the flag is now `syntaxError`, set where the error 
is added, which is what it actually tracks. Thanks, the name was genuinely 
misleading.
   
   I deliberately did not take the behaviour change. `topLevelTernary` is a 
pure string check that never consults the catalog, so a catalog that threw 
tells us nothing about whether the expression has a top-level ternary. Setting 
the flag in the catch block would suppress a correct and useful report exactly 
when validation is already degraded and the user has been given no other 
message for that line. The only reason the check is gated at all is to keep one 
message per expression, which a *reported* syntax error achieves; an exception 
produces no message, so there is nothing to collide with.
   
   A comment now records that reasoning at the call site so the asymmetry does 
not read as an oversight:
   
   ```java
   // the parser cannot report this one: the expression is valid, it just does 
not mean what it says.
   // Only a reported syntax error suppresses it, to keep one message per 
expression; a catalog that could
   // not be asked at all does not, because this check does not depend on it.
   ```
   
   Happy to switch if you would rather have the stricter gate.
   
   _Claude Code on behalf of Adriano Machado (@ammachado)_
   
   _This was generated by an AI agent and may contain inaccuracies. Please 
verify before relying on it._



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to