fix: G/END a second time scrolls the last line to the top

[VXNCXNX]

Fixes #1734.

What was lost

The non-selectable path still has the toggle you described:

} else if (this->get_top() == last_line) { this->set_top(tail_bottom);
} else if (tail_bottom <= this->get_top()) { this->set_top(last_line);
} else { this->set_top(tail_bottom); }

First press puts the last row at the bottom, second press moves the last line to the top, and it alternates from there. The is_selectable() branch had been reduced to a bare set_selection(last_line), so once the selection reached the last line every further press was a no-op.

git log -S tail_bottom traces the toggle back to 0306287 (2013), so it's the original behavior rather than something inferred.

The fix

Rather than duplicating the three cases inside the selectable branch, the two paths now share them. Only the entry condition differs: in a selectable list, the first press moves the selection; after that, presses fall through to exactly the same top-line toggle:

if (this->is_selectable()
    && this->get_selection() != std::make_optional(last_line)) {
    this->set_selection(last_line);
} else if (this->get_top() == last_line) {
    ...

I wrote it as a duplicated three-arm block first, and a review caught that it diverged from the original in one state — top strictly above tail_bottom with the selection already on the last line (reachable when the selection is off-screen below, e.g. after a filter change). There the original scrolls to show the tail first; the duplicate jumped straight to last_line. Sharing the arms removes that class of mistake rather than patching the one instance.

Simulated across every reachable state with the selection on the last line:

top:    0   2   5   9  10  15  19  20
orig:  10  10  10  10  20  20  20  10
new:   10  10  10  10  20  20  20  10

and the press sequence from a fresh view is top=10, 20, 10, 20, … with the selection pinned to the last line.

Two other details:

• std::make_optional is explicit because get_selection() returns an optional, and a bare != last_line treats "no selection" as "selection is elsewhere". Correct here by accident, but not something to leave to the reader.
• An empty list makes last_line == -1, so there's now an early break. Previously that fell into set_selection(-1), which fired a spurious listview_selection_changed.

Verification, and its limit

I could not run lnav. autoconf/automake/cmake/pkg-config are absent from my environment, so there's no full build and no make check, and this is an interactive TUI behavior besides.

What I did do is compile the translation unit against the repo's own bundled headers, which does work:

g++ -fsyntax-only -std=c++17 -DPCRE2_CODE_UNIT_WIDTH=8 \
    -I src -I src/fmtlib -I src/third-party \
    -I src/third-party/notcurses/include -I src/third-party/date/include ...

clean with the patch, and it rejects a deliberately introduced typo, so it's a real check rather than a no-op. The state-space comparison above was done by simulating both branches.

Someone able to press G twice in a real lnav would be worth more than either.

NEWS.md entry added under v0.14.1.

[tstack]

I could not run lnav. autoconf/automake/cmake/pkg-config are absent from my environment, so there's no full build and no make check, and this is an interactive TUI behavior besides.

This is not acceptable, get out of here.