fix: arrange dependent expansions accordingly

[henrebotha]

Commit a97788d introduces a simple error by moving the definition of ABBR_LINE_CURSOR_MARKER below where it's read (by the ABBR_EXPANSION_CURSOR_MARKER definition).

[henrebotha]

Not sure which branch to target — I assume either next or v6, but let me know if I should change it.

[henrebotha]

Also, wasn't sure how to write a test for this. All tests pass before and after this change.

[olets]

Good catch, thanks! Fix looks right.

Not sure which branch to target

main please. If into next, merging into main will be blocked by waiting for me to cut the next release (low bar, but still… don't want to be the blocker). If into v6, merging will into main will be blocked by releasing v6 (higher bar, could be a long wait).

a test for this

Off the top of my head, untested, it will be:

• In tests/index
  • where user configs are saved, save the user's ABBR_LINE_CURSOR_MARKER and ABBR_EXPANSION_CURSOR_MARKER
  • where test context is set, unset ABBR_EXPANSION_CURSOR_MARKER, and set ABBR_LINE_CURSOR_MARKER to something not the default (maybe %%?)
  • where user config is restored, restore the user's ABBR_LINE_CURSOR_MARKER and ABBR_EXPANSION_CURSOR_MARKER
• Add tests/abbr-config (breaks the pattern of the files being named for subcommands, but oh well), which has one test for whether ABBR_LINE_CURSOR_MARKER and ABBR_EXPANSION_CURSOR_MARKER are the same

If you're up for adding that, that'd be great!

[henrebotha]

The test to catch the bug needs to actually unset both parameters, because the error only occurs when ABBR_EXPANSION_CURSOR_MARKER tries to default to the value of ABBR_LINE_CURSOR_MARKER before the latter is set. So I just added a test case for that instead.

[olets]

Oh hm I just tried the new ./tests in the main branch, and the new test passes when it should fail.

% git checkout main
% git checkout <this PR's branch> -- tests
% exec zsh
% . ./tests/index.ztr.zsh config
PASS …

I haven't investigated.

[henrebotha]

Haha, this is silly.

The commit that introduces the error is a97788d, which is not yet merged into main. So basing my fix on main is a little senseless; looking at the diff here, the actual change I made is not changing the ordering of the two relevant parameters, just shifting them down in the file. It's only in a97788d that the two parameters' order is reversed, introducing the error.

So base on next then?

[olets]

Oh! Ha. Yes base on next. Which I force-pushed yesterday, so make sure to pull first. I always use --with-lease so it should be painless, but this might be the first time I've seen it put to the test.

[henrebotha]

Rebased onto latest next. Do I point the MR at next or main?

[olets]

next. And I'll merge next into main asap

[henrebotha]

Alright, rebased again one last time for old times' sake and force-pushed. MR now points at next and contains only a single commit. Please merge at your convenience.