gnodet commented on PR #12038:
URL: https://github.com/apache/maven/pull/12038#issuecomment-4475287787
@blaspat Thanks for applying the suggestions! Two items still need to be
addressed before this can be merged:
**1. `&` is still in the `X=` lookahead (line 139)**
`|| c == '&'` needs to be removed from the `>=`/`<=`/`==`/`!=` condition.
Currently `&=` would be tokenized as a valid two-char operator, but it isn't
one. The `&&` handling in the `else if` below is correct — it's only the `X=`
group that needs the fix.
Current:
```java
if ((c == '>' || c == '<' || c == '=' || c == '!' || c == '&')
&& i + 1 < expression.length()
&& expression.charAt(i + 1) == '=') {
```
Should be:
```java
if ((c == '>' || c == '<' || c == '=' || c == '!')
&& i + 1 < expression.length()
&& expression.charAt(i + 1) == '=') {
```
**2. `||` tokenization is missing — this is now a regression**
`|` was added to the delimiter list (good), but there's no branch to combine
two consecutive `|` chars into a single `||` token. This means `a || b` now
tokenizes as `["a", "|", "|", "b"]` instead of `["a", "||", "b"]`, which
**breaks `||` entirely** — even with spaces, which worked before this PR.
Please add a `||` branch after the `&&` one:
```java
} else if (c == '&' && i + 1 < expression.length() && expression.charAt(i +
1) == '&') {
tokens.add("&&");
i++; // Skip the next character
} else if (c == '|' && i + 1 < expression.length() && expression.charAt(i +
1) == '|') {
tokens.add("||");
i++; // Skip the next character
} else {
```
**3. Add `||` multiline tests** (as mentioned in the earlier review comment)
_Claude Code on behalf of Guillaume Nodet_
--
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]
