bneradt commented on PR #12989:
URL: https://github.com/apache/trafficserver/pull/12989#issuecomment-4093532427
Codex review:
I have two follow-up concerns after reviewing and validating this PR.
1. `SESSION_VARS` does not look wired into the LSP declaration scanner yet.
`tools/hrw4u/src/lsp/strings.py` still only enters declaration mode for `VARS {
... }`, so once users adopt `SESSION_VARS`, session-scoped variables appear
likely to lose normal hover/type metadata in `hrw4u-lsp`. I have not done a
live LSP repro yet, so I am flagging this as a tooling concern rather than a
confirmed runtime bug.
2. `hrw4u-kg` skips `SESSION_VARS`. In `tools/hrw4u/src/kg_visitor.py`,
`visitSection()` handles `ctx.varSection()` but not `ctx.sessionVarSection()`,
so the new declaration block is ignored by knowledge-graph extraction. I
reproduced this in the `fix_tickets_4` container by running `hrw4u-kg` on
`tools/hrw4u/tests/data/vars/session_assign.input.txt`: the output contained
the `VARS` block and `bool_0`, but dropped the `SESSION_VARS` block and its
session variables entirely.
--
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]
