jarun / nnn

n³ The unorthodox terminal file manager
BSD 2-Clause "Simplified" License
19.22k stars 761 forks source link

edit sel not working #446

Closed jgmtzmd closed 4 years ago

jgmtzmd commented 4 years ago

well after updating to 2.9 on my open suse leap 42.1 the "edit sel" function just stopped working, now it only opens the selection file (~/.config/nnn/.selection) not the paths listed in it.

jarun commented 4 years ago

Are you saying there's no path listed in the selection file?

jgmtzmd commented 4 years ago

there is a path listed. the problem is that pressing the 'e' key opens the selection file when it should open the file listed in it.

-- Enviado desde myMail para Android

martes, 21 enero 2020, 07:45p. m. -06:00 de Mischievous Meerkat notifications@github.commailto:notifications@github.com:

Are you saying there's no path listed in the selection file?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/jarun/nnn/issues/446?email_source=notifications&email_token=AGFRKUIUGTMRIEQGUXCB3TLQ66QLRA5CNFSM4KJ5K35KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJR5Y4I#issuecomment-576969841, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGFRKUMWAQTO53MC3IXBO2LQ66QLRANCNFSM4KJ5K35A.

jarun commented 4 years ago

OK. Edit in EDITOR and open in PAGER are gone in v2.9. You can have keybinds for those. See https://github.com/jarun/nnn/tree/master/plugins#some-useful-key-command-examples

jgmtzmd commented 4 years ago

ok. thanks. the help that is shown after pressing "?" still mentions 'e' for edit sel. that caused my confusion.

-- Enviado desde myMail para Android

martes, 21 enero 2020, 08:08p. m. -06:00 de Mischievous Meerkat notifications@github.commailto:notifications@github.com:

OK. Edit in EDITOR and open in PAGER are gone in v2.9. You can have keybinds for those. See https://github.com/jarun/nnn/tree/master/plugins#some-useful-key-command-examples

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/jarun/nnn/issues/446?email_source=notifications&email_token=AGFRKUOEOZHYXEMSVFI6453Q66TCFA5CNFSM4KJ5K35KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJR67RY#issuecomment-576974791, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGFRKUM2XP7I2W5YP62LJJDQ66TCFANCNFSM4KJ5K35A.

jarun commented 4 years ago

It stands for "edit selection", not "edit individual files in selection". :+1:

ghost commented 4 years ago

Just to confirm, is it now impossible to bind e to open in EDITOR? It seems like with keybinds I have to input ;e.

jarun commented 4 years ago

Or you can change the code and compile.

0xACE commented 4 years ago

Just to confirm, is it now impossible to bind e to open in EDITOR? It seems like with keybinds I have to input ;e.

commit b9f0db827a8643e73d654115e458e3e8bde624c7
Author: 0xACE <null>
Date:   Fri Jan 17 00:06:27 2020 +0100

    Legacy patches

    contains:
    SEL_RUNEDIT
    SEL_RUNPAGE
    SEL_EXEC

    SEL_CP

diff --git a/src/nnn.c b/src/nnn.c
index 39c9b96..dab7a81 100644
--- a/src/nnn.c
+++ b/src/nnn.c
@@ -3450,6 +3450,29 @@ static void find_accessible_parent(char *path, char *newpath, char *lastname, in
    xdelay(XDELAY_INTERVAL_MS);
 }

+static bool execute_file(int cur, char *path, char *newpath, int *presel)
+{
+   if (!ndents)
+       return FALSE;
+
+   /* Check if this is a directory */
+   if (!S_ISREG(dents[cur].mode)) {
+       printwait("not regular file", presel);
+       return FALSE;
+   }
+
+   /* Check if file is executable */
+   if (!(dents[cur].mode & 0100)) {
+       printwait("permission denied", presel);
+       return FALSE;
+   }
+
+   mkpath(path, dents[cur].name, newpath);
+   spawn(newpath, NULL, NULL, path, F_NORMAL);
+
+   return TRUE;
+}
+
 /* Create non-existent parents and a file or dir */
 static bool xmktree(char* path, bool dir)
 {
@@ -5209,12 +5232,16 @@ nochange:
        case SEL_REDRAW: // fallthrough
        case SEL_RENAMEMUL: // fallthrough
        case SEL_HELP: // fallthrough
+       case SEL_RUNEDIT: // fallthrough
+       case SEL_RUNPAGE: // fallthrough
        case SEL_LOCK:
        {
            bool refresh = FALSE;

            if (ndents)
                mkpath(path, dents[cur].name, newpath);
+           else if (sel == SEL_RUNEDIT || sel == SEL_RUNPAGE)
+               break;

            switch (sel) {
@@ -5235,6 +5262,12 @@ nochange:
                if (cfg.filtermode)
                    presel = FILTER;
                continue;
+           case SEL_RUNEDIT:
+               spawn(editor, dents[cur].name, NULL, path, F_CLI);
+               continue;
+           case SEL_RUNPAGE:
+               spawn(pager, dents[cur].name, NULL, path, F_CLI);
+               continue;
            default: /* SEL_LOCK */
                lock_terminal();
                break;
@@ -5664,12 +5697,17 @@ nochange:
            setdirwatch();
            clearfilter();
            goto begin;
+       case SEL_EXEC: // fallthrough
        case SEL_SHELL: // fallthrough
        case SEL_LAUNCH: // fallthrough
        case SEL_RUNCMD:
            endselection();

            switch (sel) {
+           case SEL_EXEC:
+               if (!execute_file(cur, path, newpath, &presel))
+                   goto nochange;
+               break;
            case SEL_SHELL:
                setenv(envs[ENV_NCUR], (ndents ? dents[cur].name : ""), 1);
                spawn(shell, NULL, NULL, path, F_CLI);
diff --git a/src/nnn.h b/src/nnn.h
index 70464fe..1cbb869 100644
--- a/src/nnn.h
+++ b/src/nnn.h
@@ -86,9 +86,12 @@ enum action {
    SEL_UMOUNT,
    SEL_HELP,
    SEL_PLUGIN,
+   SEL_EXEC,
    SEL_SHELL,
    SEL_LAUNCH,
    SEL_RUNCMD,
+   SEL_RUNEDIT,
+   SEL_RUNPAGE,
    SEL_LOCK,
    SEL_SESSIONS,
    SEL_QUITCTX,
@@ -196,9 +199,9 @@ static struct key bindings[] = {
    /* Select all files in current dir */
    { 'a',            SEL_SELALL },
    /* List, edit selection */
-   { 'e',            SEL_SELEDIT },
+   { 'y',            SEL_SELEDIT },
    /* Copy from selection buffer */
-   { 'p',            SEL_CP },
+   { 'P',            SEL_CP },
    { CONTROL('P'),   SEL_CP },
    /* Move from selection buffer */
    { 'v',            SEL_MV },
@@ -228,6 +231,8 @@ static struct key bindings[] = {
    /* Run a plugin */
    { ';',            SEL_PLUGIN },
    { CONTROL('S'),   SEL_PLUGIN },
+   /* Execute file */
+   { 'C',            SEL_EXEC },
    /* Run command */
    { '!',            SEL_SHELL },
    { CONTROL(']'),   SEL_SHELL },
@@ -236,6 +241,10 @@ static struct key bindings[] = {
    /* Run a command */
    { ']',            SEL_RUNCMD },
    { CONTROL('P'),   SEL_RUNCMD },
+   /* Open in EDITOR or PAGER */
+   { 'e',            SEL_RUNEDIT },
+   { 'E',            SEL_RUNEDIT },
+   { 'p',            SEL_RUNPAGE },
    /* Lock screen */
    // { '0',            SEL_LOCK },
    /* Quit a context */

Maybe one day i'll write a funciton to let you bind plugins to regular keys via something like this:

switch(sel) { 
 char c = <w/e to get the character of the sel>;
 <call SEL_PLUGIN with argument>
ghost commented 4 years ago

So I've been using the new keybindings for a while now, and honestly, it really destroyed usability for me. I'm going to stick to a personal fork or just use older versions for now.

jarun commented 4 years ago

it really destroyed usability for me

That's a big statement. How so?

0xACE commented 4 years ago

That's a big statement. How so?

I'd guess it's that users are resentful to change.

I feel bad that I was unavailable during the discussions of standardizing the keys for nnn, so instead of complaining, I've remained quiet to patched things locally.

A couple of nit picks I can think off the top of my head:

There are probably more, but since these changes has been made: I haven't had much computer time lately, so my testing hasn't been thorough. Also my main computer crashed badly, so I'm dealing with other struggles right now, but I try to keep up with nnn every few days.

User has to rehabilitate this: means that the user now has a responsibility to keep up with the product development. As the product they have used has changed and their knowledge isn't applicable anymore.

But I do understand the judgement on "standardizing" it.

Though I'm also interested in what @billop has to say.

jarun commented 4 years ago

Adding @maximbaz.

We had to standardize at some point. This was done exactly to stop changing keybinds on every release.

Yes, it will take some time to get used to older keys (like x and , you mentioned) with new changes but it can't be more than a few days. I also had to go through it.

x confirms OR trashes, p copies (you can recover), v copies (you can recover).

0xACE commented 4 years ago

We had to standardize at some point. This was done exactly to stop changing keybinds on every release.

Yeah there is no arguing that.

My main concern atm is x, v and p should be shielded from accidental misfire. Those three commands are verbs that can act upon the file structure. They are damaging/writing commands which should be behind safeguards.

Yes I'm aware of it asking for confirmation, it just feels like walking on a ledge without rails. The question is who is the first user to misfire xy? (Luckily it's a unlikely combination to hit)

Another one that I should've mentioned was times where I hit c and type my host and not realising it was prompting me for rclone/sshfs, leading to another confusing moment as I type in my designated hostname and potentially breaking things. In my local copy i patched out rclone so it is no longer an issue for me and therefore I forgot to mention it.

But you can consider my opinion on this topic to be void. As I don't believe my usage is representative of what's best for nnn at this point.

TL:DR; don't waste your time on my answer, see what the end users has to say.

jarun commented 4 years ago

In my local copy i patched out rclone so it is no longer an issue for me and therefore I forgot to mention it.

We can make it smart enough to detect if rclone is present or not.

0xACE commented 4 years ago

We can make it smart enough to detect if rclone is present or not.

I guess so. But really I wouldn't put much attention to it. just put it up on the TODO and see if anyone ever bothers to touch it. The problem may only be me. and in remote_mount() opt = 's' on my branch so the issue is gone.

Mostly I'm saying this because I know this will be another merge conflict for me :D

ghost commented 4 years ago

it really destroyed usability for me

That's a big statement. How so?

Using nnn with neovim is such a big part of my workflow, removing the open with EDITOR keybinding made it quite unusuable for me. With the plugin setting I thought it would be twice as much work (2 keys instead of 1) but it's been actually worse since it doesn't work if there's a message or something (happened almost every time, since I would accidently press 'e', then nnn would complain ("0 selected"), then I would press ';e', but for some reason the presence of that message eats up the ';' so it really inputs 'e' which gives me that message again, so then I press escape to get rid of that message so that I can input ';e'.

So best case scenario: ';e' but in reality: 'e;e\<esc>;e' worst case (first day): 'e;e;e;e;e;e;e;e;e;e;e;e;e...'

But I think making the move and copy bindings lowercase is a big improvement, and much more intuitive.

I'm kind of disillusioned at this point honestly of not reading config files and using header files or env vars, I understand the reasoning (all suckless software is like this) but when a keybinding changes without warning it's such a pain.

Sorry for the rant but I hope I gave some useful feedback there.

jarun commented 4 years ago

but for some reason the presence of that message eats up the ';'

I see. That's a problem.

when a keybinding changes without warning it's such a pain.

Yes, and it was an issue with several releases so far. So in v2.9 we discussed the keybinds are now locked. See issue #422.

jarun commented 4 years ago

@maximbaz we had several usability reports over the re-assignment of key e. And the above problem makes it worse. I am thinking of making an exception for e given the number of user reports and return it back to edit in EDITOR. However, there won't be any open in PAGER still...

We need a keybind for edit selection though. What do you suggest? @billop please feel free to pitch in.

maximbaz commented 4 years ago

Before we jump to action, I wanted to confirm something, @billop have you tried ~exporting NNN_USE_EDITOR=1~ running nnn with -e flag and using Enter instead of now gone e? That's exactly what I ended up doing, yes it requires knowing that e was remapped, but at least for me it was quite easy to switch to using Enter once I knew that.

maximbaz commented 4 years ago

but for some reason the presence of that message eats up the ';'

I see. That's a problem.

can we fix this? don't eat the key press while popup is shown, but move it down the global key handling pipeline maybe?

jarun commented 4 years ago

Yes, please! I am looking into another issue right now. Please take care of this.

jarun commented 4 years ago

@maximbaz have you tried exporting NNN_USE_EDITOR=1 and using Enter instead of now gone e

There are cases. I am using a custom cli-only opener (-c) which overrides -e. This is done so the opener can still use it's own defaults. So I have glow to open markdown, man to open man page and w3m to open html.

In case of the man page, I want to view, edit, view and so on... so I do need a editor keybind. I use the nav-as-you-type mode continuously so the ;e suits me well. However, for regualr users e was probably very convenient.

maximbaz commented 4 years ago

That's what I'm trying to understand, maybe the reason users request e is simply because they are not aware that Enter can do the job :wink:

In neovim I imagine you don't need custom opener, so in that specific workflow Enter could be just as convenient as e.

All I'm saying is perhaps lets get more concrete feedback before we rush and change hotkeys right now, especially because we sort of promised ourselves that they will be frozen :slightly_smiling_face:

jarun commented 4 years ago

because they are not aware that Enter can do the job

No it won't.

This is done so the opener can still use it's own defaults. So I have glow to open markdown, man to open man page and w3m to open html.

The custom opener opens in these utilities when you press Enter.

jarun commented 4 years ago

@billop

but it's been actually worse since it doesn't work if there's a message or something (happened almost every time

This is fixed at commit c0f423496ea7b0d149bb64324e6590049e97d561.

Do you still see the need for the additional key for edit in EDITOR?

jarun commented 4 years ago

@KlzXS what's your opinion on this? Shall we have the e for edit in EDITOR back or you think it's sufficient to have users set a run cmd as plugin...

KlzXS commented 4 years ago

I think we should have e be edit file as it is a bit more common to edit a file rather than selection.

You can put edit selection as E or move it somewhere else completely.

jarun commented 4 years ago

You can put edit selection as E or move it somewhere else completely.

I was thinking of the same keybind.

maximbaz commented 4 years ago

If we have e keybind to edit file, do I understand correctly that the purpose of -e CLI flag is simply to make Enter keypress do exactly what e keypress does? If so, does it even make sense to have -e CLI flag, why not simply teach users to edit with e always? Or there is more to it?

jarun commented 4 years ago

So having the key e helps when you want to actually edit the .md in EDITOR instead of viewing it in glow.

Another use case is - after creating an empty file, if you want to edit it, pressing Enter will not work. Currently you will have to use open with. However, if you have e you can edit it in EDITOR.

jarun commented 4 years ago

And the reason I would prefer E (capital) for edit sel is - it gives the message - do not abuse it. ;)

maximbaz commented 4 years ago

ok, I'm fine with the change, let's have the e back.

jarun commented 4 years ago

Thanks for the inputs guys!