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]