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

sorting state is shared between contexts #1757

Closed
7 of 9 tasks
azuline opened this issue Nov 10, 2023 · 2 comments · Fixed by #1762
Closed
7 of 9 tasks

sorting state is shared between contexts #1757

azuline opened this issue Nov 10, 2023 · 2 comments · Fixed by #1762
Labels

Comments

@azuline
Copy link
Contributor

azuline commented Nov 10, 2023

Environment details

  • Operating System: NixOS
  • Desktop Environment: none+i3
  • Terminal Emulator: kitty
  • Shell: fish
  • Custom desktop opener (if applicable):
  • Program options used: aAERx
  • Configuration options set: see https://github.com/azuline/nixos/blob/master/home/nnn/default.nix#L24
  • Plugins are installed
  • Issue exists on nnn master: pretty sure, i might be a few commits behind the latest commit

Exact steps to reproduce the issue

When I change contexts, the sort order of the current context is preserved in the second context. This preservation occurs when I set the sort order by hand (video 1), but also when the sort order is part of the NNN_ORDER envvar (video 2). Even if case 1 is intentional, I do not believe case 2 is, as leaving the NNN_ORDER-ed directory in the same context unsets the sort and does not preserve it.

There is another strange behavior: If i unset the sorting state in the 2nd context, go back to the first context, and then attempt to re-set the sort, nothing happens. It is as if my first attempt to re-set the sort actually performed the unset operation in the first context, and the second attempt actually did re-set the sort (though I'm not too sure what's really going on in the code).

Video 1: https://github.com/jarun/nnn/assets/51880422/e56ac3ac-94cd-41ef-89cf-31c56798c202
Video 2: https://github.com/jarun/nnn/assets/51880422/50536d6c-3f52-4618-8c0b-bd3c3b66a4eb

What is the desired behavior? I'm happy to open a pull request, but I don't want to implement the wrong behavior ^_^

@azuline azuline added the bug label Nov 10, 2023
@jarun
Copy link
Owner

jarun commented Nov 17, 2023

I tried the following:

Sort manually by size (s) and switch to an ordered context. In this case the sort order is not changed in the second context.
However, if I manually set the reverse order (r) or version sort (v), the reverse order is added in the second context too.

The second observations seems to be a manifestation of the same. If you check set_sort_flags(), r and v use special function pointers. It seems they are retained and adds up along with the order dictated by NNN_ORDER even for a context that is already open.

The issue maybe somewhere in the below lines:

6838         if (order && cd) {
6839                 if (cfgsort[cfg.curctx] != '0') {
6840                         if (cfgsort[cfg.curctx] == 'z')
6841                                 set_sort_flags('c');
6842                         if ((!cfgsort[cfg.curctx] || (cfgsort[cfg.curctx] == 'c'))
6843                             && ((r = get_kv_key(order, path, maxorder, NNN_ORDER)) > 0)) { // NOLINT
6844                                 set_sort_flags(r);                                                                 
6845                                 cfgsort[cfg.curctx] = 'z';
6846                         }
6847                 } else
6848                         cfgsort[cfg.curctx] = cfgsort[CTX_MAX];
6849         }

Please have a look and raise a PR.

@azuline
Copy link
Contributor Author

azuline commented Nov 17, 2023

thanks, took a look and put up a pull request!

@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants