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

picker & fifomode: on 'l' press, pick file instead of opening it #1832

Closed
wants to merge 1 commit into from

Conversation

Oxore
Copy link

@Oxore Oxore commented Feb 25, 2024

This enables a user to pick files by pressing l key when using nnn as a picker or in fifomode, which is used by vim plugins such as mcchrish/nnn.vim and luukvbaal/nnn.nvim. It also prevents nnn from opening a file when l key is pressed, which is unexpected and unwanted behavior form the user's perspective (IMO).

I explained the use case in #1827 (so not going to copy-paste or rewrite it here) and also tried to discuss whether it is acceptable to make nnn pick files by pressing l in a way which I am presenting in this PR, but I guess it turned out to be vague, so I decided to straight file a PR to show what I want.

@N-R-K
Copy link
Collaborator

N-R-K commented Feb 25, 2024

I remember asking for something similar a couple years ago but @jarun didn't like it and said there was some reason to not do it. Don't remember the specifics on why.

@Oxore
Copy link
Author

Oxore commented Feb 25, 2024

I am really looking forward to know why it shouldn't be done this way or shouldn't be done at all.

@jarun
Copy link
Owner

jarun commented Mar 5, 2024

From https://github.com/mcchrish/nnn.vim#explorer:

Pressing l or Right on a file would open it instead of picking. Use -o via nnn#command to disable this.

Any change to l also does the same with Right. I am fine with l. But does picking a file with Right seem to be the correct behaviour?

@Oxore
Copy link
Author

Oxore commented Mar 6, 2024

Any change to l also does the same with Right. I am fine with l. But does picking a file with Right seem to be the correct behaviour?

At least this behaviour is consistent with the usual navigation mode. If you ask me, I think this is correct.

From https://github.com/mcchrish/nnn.vim#explorer:

Pressing l or Right on a file would open it instead of picking. Use -o via nnn#command to disable this.

Uh-oh... I just realized that the current implementation in the PR renders the -o option ineffective in the picker/fifomode. Assuming that the use case of the -o option is not only for the picker/fifomode, this is an obvious conflict. I will update the PR to fix this conflict.

@Oxore
Copy link
Author

Oxore commented Mar 6, 2024

/root/project/src/nnn.c:6768:13: error: function 'browse' exceeds recommended size/complexity thresholds [readability-function-size,-warnings-as-errors]
static bool browse(char *ipath, const char *session, int pkey)
            ^
/root/project/src/nnn.c:6768:13: note: 953 statements (threshold 950)

Apparently the browse function is perfectly balanced in terms of statement count and I just exceeded the quota, according to the CI. Don't know what to do with this at the moment, but I find it funny.

@@ -7139,13 +7139,19 @@ static bool browse(char *ipath, const char *session, int pkey)
}

#ifndef NOFIFO
if (g_state.fifomode && (sel == SEL_OPEN)) {
if (g_state.fifomode) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does one open the file when fifomode is enabled?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is made impossible on purpose, as a side effect. The story is the same as with picker mode, as I explained in #1832 (comment), but the use case of fifomode is a bit narrower: to embed into other software as a picker, that is being open constantly. I can't see why one would need to open a file in such case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this is just my understanding. There may be other use cases and opinions.

/* If opened as vim plugin and Enter/^M pressed, pick */
if (g_state.picker && (sel == SEL_OPEN)) {
/* If opened as vim plugin and Enter/^M/Double-click or l/Right pressed, pick */
if (g_state.picker) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does one open the file in picker mode?

Copy link
Author

@Oxore Oxore Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to open a file in picker mode. Why should it be? It is not a rhetorical question, I would really like to know why.

As far as I understand, the use case of picker mode is akin to tools like FZF and Rofi, but tailored to file picking by providing a comprehensive file system navigation. I don't see why one would need to open or edit a file while in this mode.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to know why.

Because it's convenient to be able to open a file from that mode. Because nnn is after all a file manager.
Say you are editing a file in vim and you want to listen to some music. You can use the plugin in picker mode and play it.

The change breaks that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the explanation! At least now I know that there is a use case and therefore the breaking change is unacceptable. I will think about implementing this behavior as an option.

@Oxore
Copy link
Author

Oxore commented Mar 30, 2024

It turns out there is a use case for opening a file in picker mode (see #1832 (comment)) and this PR breaks it. I would like to close this PR for now. I should think about making the new behavior optional.

@Oxore Oxore closed this Mar 30, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2024
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.

None yet

3 participants