fix: do not offer SQL completions after the statement terminator

[shivampwm2020]

Problem

Fixes #1751.

In SQL mode, running a statement and then typing the same statement again
leaves a completion popup open on the trailing ;. Pressing Enter then accepts
the suggestion instead of executing the query, corrupting the input:

select * from caddy_log where 1=1;   -- runs fine the first time
select * from caddy_log where 1=1;   -- 2nd time: suggestions appear after ';'

Root cause

annotate_sql_statement() tokenizes the line and has no rule for ;, so it
falls through to the catch-all (\A.) pattern and tags the terminator with
SQL_GARBAGE_ATTR. The SQL completion trigger in rl_sql_change()
(src/readline_callbacks.cc) then looks up the attribute under the cursor and
only suppressed completion for SQL_NUMBER_ATTR / SQL_HEX_LIT_ATTR:

} else if (is_req
           || (attr_iter->sa_type != &SQL_NUMBER_ATTR
               && attr_iter->sa_type != &SQL_HEX_LIT_ATTR)
           || rc.tc_popup_type == textinput_curses::popup_type_t::completion)

That explains the "first time works, second time breaks" behaviour:

• First run — the schema has not been learned yet, so the completion set is
  empty and open_popup_for_completion() returns without showing anything
  (it bails when possibilities.empty()). Enter falls through to
  on_perform() and the query executes.
• After that — the log format is registered and its columns are known, so
  similar_to(";", 10) now yields candidates. The popup opens, and
  NCKEY_ENTER in textinput_curses takes the "accept completion" branch
  whenever tc_popup.is_visible(), so Enter never reaches on_perform().

The terminator is a token boundary with nothing to complete, so the garbage
annotation is the right thing to skip.

Fix

Skip completion when the attribute under the cursor is SQL_GARBAGE_ATTR:

} else if (attr_iter->sa_type != &SQL_GARBAGE_ATTR
           && (is_req
               || (attr_iter->sa_type != &SQL_NUMBER_ATTR
                   && attr_iter->sa_type != &SQL_HEX_LIT_ATTR)
               || rc.tc_popup_type == textinput_curses::popup_type_t::completion))

The parenthesisation matters — a bare a && b || c || d binds as
(a && b) || c || d and leaves the bug in place. I verified that failure mode
explicitly (see Testing).

The guard applies to explicit requests too: the ; has no completions worth
offering, so no popup is shown there in any mode.

The annotator itself is left alone — ; → sql_garbage is existing, tested
behaviour (see the sql_garbage entries in test/expected/test_sql_anno.sh_*)
that sql.formatter.cc and cmd.parser.cc also rely on.

Testing

Honest note on build/test coverage: I could not complete a full lnav build
or run the ctest suite in my environment — it lacks root access and the
required development headers (libarchive-dev, libsqlite3-dev,
libpcre2-dev, libcurl4-*-dev, notcurses, vendored submodules), so
cmake fails at find_package/#include <pcre2.h>/notcurses/notcurses.h.
The verification below is what I could actually run; the golden-file test case
I added has not been executed.

What I did verify:

1. Patch shape. Bracket/paren balance of the edited function is identical
   to the original, so no structural damage was introduced.
2. Predicate harness. Extracted the exact condition into a standalone
   translation unit (g++ -std=c++20 -Wall -Wextra, no lnav deps) and asserted
   the truth table. All assertions pass. Coverage: sql_garbage at the
   cursor no longer completes in any mode; identifiers/keywords still
   complete; the pre-existing number/hex-literal suppression is unchanged.
3. Negative controls. Removing the guard re-introduces the bug, and
   writing it as a && b || c || d also fails the harness — confirming both
   the bug and the need for the explicit grouping.
4. Server-side diff. Comparing the pushed branch against upstream master
   reports ahead_by: 1, behind_by: 0, two files changed (11 + 5 lines),
   and the patch content matches the intended edit.

Added to test/test_sql_anno.sh:

run_cap_test ./drive_sql_anno "SELECT * FROM caddy_log WHERE 1=1;"

This pins the annotation the fix depends on (; must stay sql_garbage).
Note that run_cap_test hashes the assertion into the expected-file name, so
the corresponding test/expected/ golden files need to be generated by
running the suite — I have not generated or committed them, since that
requires the build above. Maintainers will need to run the suite (or tell me
and I'll add the golden files) before this case can pass.

Signed-off-by: Shivam Maurya shivam@programmingwithmaurya.com