Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restore sort function pointers when restoring cfg #1762

Merged
merged 2 commits into from
Nov 19, 2023
Merged

Conversation

azuline
Copy link
Contributor

@azuline azuline commented Nov 17, 2023

closes #1757

This PR fixes a sorting synchronization issue.

The root cause of the bug in #1757 is that the sorting function entrycmpfn is not restored when changing contexts, causing reverse (and version) to be "global" in that setting them on one context caused them to be set on all contexts.

This PR fixes the bug by setting entrycmpfn and namecmpfn to the correct pointers whenever cfg is set.


simulation of state variables in the bugged scenario:

  1. starting state
    • cfg0.reverse = 0, entrycmpfn = &entrycmp
  2. set context 1 to r
    • cfg0.reverse = 1, entrycmpfn = &reventrycmp
  3. create context 2. context 2 should default to r.
    • cfg0.reverse = 1, cfg1.reverse = 1, entrycmpfn = &reventrycmp
  4. unset r in context 2.
    • cfg0.reverse = 1, cfg1.reverse = 0, entrycmpfn = &entrycmp
  5. switch back to context 1. context 1 should not appear to be r.
    • cfg0.reverse = 1, cfg1.reverse = 0, entrycmpfn = &entrycmp
  6. try to set r again. no-ops when it should do something.
    • cfg0.reverse = 0, cfg1.reverse = 0, entrycmpfn = &entrycmp

the "double unset" behavior is because the statusbar uses a function pointer comparison to print R and V. it does not use cfg.reverse or cfg.version. if the statusbar used cfg.reverse, we would see R in the statusbar, yet no reverse sort in the entrylist.

video :)

nnn-bugfix.mp4

src/nnn.c Outdated Show resolved Hide resolved
src/nnn.c Outdated Show resolved Hide resolved
@azuline azuline requested a review from jarun November 18, 2023 21:33
@jarun jarun merged commit 744a755 into jarun:master Nov 19, 2023
7 checks passed
@jarun
Copy link
Owner

jarun commented Nov 19, 2023

Thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2023
@azuline azuline deleted the sortctx branch April 16, 2024 03:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sorting state is shared between contexts
2 participants