Don't hardcode keys (+, -, \, *, X-enter)

[mooffie]

midnight.c:midnight_callback() hardcodes several keys:

• {alt,ctrl,ctrl-shift}-enter
• +, -, \, * (for selecting files)

These two patches fix this. The advantages are explained below.

[mooffie]

The 1st patch: It handles the {alt,ctrl,ctrl-shift}-enter key combinations.

It installs these keys on the [main] keymap.

(I've just found ticket #3075, which asks to fix this. It'd be easier to mark that other ticket as a duplicate of this one than the other way around.)

Notes:

• I've verified that MC indeed translates the strings "ctrl-enter" etc. to the values we use now (KEY_M_CTRL | '\n', etc).)

• Keymap [main] is used in MSG_UNHANDLED_KEY, after the panel widgets handle the key, and before the input widget handles it. I don't see a problem in that.

[mooffie]

The 2nd patch: It handles +, -, \, *.

It does this by introducing a new keymap, [main:empty-cmdline], that's only effective when the command-line is empty. The advantages are two:

• The user is able to customize the keys. E.g., @telep ​wants to use '\' to switch panels.

• The user is able to add more commands. E.g., @ossi ​suggests to use "&" for some command. Whoever wants can bind . , ` = : ! [ ] etc. etc. to whatever commands he wishes! E.g., you can bind , (comma) to chmod.

The patch also fixes the description for the only_leading_plus_minus option.

[mooffie]

(The patches are independent of one another.)

[mooffie]

To test the patches, create ~/.config/mc/mc.keymap containing (for example):

[main:empty-cmdline]
Select = prime                    # Select files with ` instead of +
ExternalPanelize = exclamation    # Use ! to bring up the panelizer.

[main]
PutCurrentSelected = ctrl-]          # Put the selected file's name on the command line
PutCurrentFullSelected = alt-ctrl-]  # ditto. But put the full path.

[andrew_b]

The "Select/Unselect/SelectInvert?" operates with files in file panel. Therefore panel should handle these command. And it do that:

   3469     case CK_SelectInvert:
   3470         panel_select_invert_files (panel);
   3471         break;
   3472     case CK_Select:
   3473         panel_select_files (panel);
   3474         break;

   3478     case CK_Unselect:
   3479         panel_unselect_files (panel);
   3480         break;

I think we don't need extra section [main:empty-cmdline]. These commands should be routed to the active panel. Such routing is performed in midnight_execute_cmd():

   1317     case CK_Select:
   1318     case CK_Unselect:
   1319     case CK_SelectInvert:
   1320         res = send_message (current_panel, midnight_dlg, MSG_ACTION, command, NULL);
   1321         break;

"Select/Unselect/SelectInvert?" handlers in filemanager level are required only because of items of main menu, which is handled here.

[mooffie]

Replying to andrew_b:

I think we don't need extra section [main:empty-cmdline]. These commands
should be routed to the active panel.

Where would the check for "empty command-line" occur?

"Select/Unselect/SelectInvert??" handlers in filemanager level are
required only because of items of main menu, which is handled here.

Oh, I see.

These commands should be routed to the active panel.

(Ok. But, supposing we stick to the extra map idea (main:empty-cmdline), then, if we route everything to the panel, we'd loose the ability to use commands that are implemented by the filemanager. So I suggest we first send these commands to the filemanager and then (if MSG_NOT_HANDLED) to the panel.)

[mooffie]

Replying to andrew_b:

I think we don't need extra section [main:empty-cmdline].

There's an issue that I didn't plan to mention (I thought it'd be clear), but seeing that we're in an "email style" correspondence it's better not to delay it:

While I marked this ticket as "task", it's also a feature. I think this proposed feature, of expanding the repertoire of keys available for us (namely: punctuation chars) to call up commands, would be a boon to some users. It's a usability issue: Not everyone is very comfortable with combination keys. I'm not referring only to people with outright disabilities. MSD conditions are mainstream nowadays. This idea, of enhanced usability, is in fact what motivated me most to work on the patch.

But let's focus right now on the question I asked in my previous comment ("Where would the check for [...]").

[andrew_b]

Replying to mooffie:

But let's focus right now on the question I asked in my previous comment ("Where would the check for [...]").

First patch moves a piece of code from midnight_callback(MSG_KEY) to midnight_execute_cmd(). I'm not sure but I hope this way may be applied for another piece of code in MSG_KEY handler.

[mooffie]

Here's a patch that doesn't use the extra keymap.

(I'm appending "v2" to this new patch's filename.)

HOWEVER, while the patch works, we won't be able to use it. The patch suffers from an unfortunate problem:

It lists the keybindings on the [main] keymap, in this way:

[main]
...
Select = kpplus; plus
Unselect = kpminus; minus; backslash
SelectInvert = kpasterisk; asterisk

The problem is that if the user already has a ~/.config/mc/mc.keymap which was copied from /etc/mc/mc.keymap of an older MC, he'd have in it:

[main]
...
Select = kpplus
Unselect = kpminus
SelectInvert = kpasterisk

This would strike off the patch's built-in bindings for plus, minus, etc., which would mean they won't work.

Do you see a solution to this problem? It looks to me as if this puts the kibosh on any solution that wants to use the [main] or [panel] keymaps.

I assume, of course, that we care about this scenario (of users having a personal mc.keymap copied whole from the original). Then again, maybe my assumption is wrong and we shouldn't care?

[mooffie]

(Note: comment #9 explains why this patch is a dud.)

[mooffie]

Replying to andrew_b:

Replying to mooffie:

Where would the check for "empty command-line" occur?

First patch moves a piece of code from midnight_callback(MSG_KEY) to midnight_execute_cmd(). I'm not sure but I hope this way may be applied for another piece of code in MSG_KEY handler.

This doesn't help me see a solution. Have you had a chance to think more of this?

Moreover, won't the configuration problem described in comment:9 pertain to any solution which uses the [main] or [panel] keymaps?

[mooffie]

So I think at the moment my patch is rejected because of a certain mental aversion to using the extra, [main:empty-cmdline] keymap.

I can understand this aversion. "What, are we going to create a keymap for every condition? [main:sun-is-shining]? [main:it's-past-tea-time]? What's so special about an empty command line that needs its own keymap?"

I don't see it like that. "empty-cmdline" is not a condition, it's a mode. Conditions is an open set (there are infinite). Modes is a closed set (finite; one is not allowed to invent). For 20 years we've had this one mode, "the command-line is empty", establishing itself. There's no reason not to name this legitimate boy. The mental aversion should be to not admitting its existence.

BTW, other systems too have mode-specific key bindings; e.g., Vim has :nmap, :imap, :cmap.

@Andrew: Could you please reconsider your opinion on this?

[mooffie]

(One advantage of the [main:empty-cmdline] keymap over the "v2" solution (of comment:9) is that you're not limited to only punctuation marks. You can even bind the <left> and <right> arrows. If this sounds weird, consider that the Lynx movement is based on this idea. So, for example, you can bind <right> to some command that expands the selection (e.g., SelectExt?). Or you can make <right> behave like ENTER (something the lynx motion fails to do).)

[andrew_b]

Replying to mooffie:

The 1st patch: It handles the {alt,ctrl,ctrl-shift}-enter key combinations. ==

It installs these keys on the [main] keymap.

(I've just found ticket #3075, which asks to fix this. It'd be easier to mark that other ticket as a duplicate of this one than the other way around.)

Why not move this patch to #3075? I don't have any questions about the patch and we close #3075.

[mooffie]

Replying to andrew_b:

Replying to mooffie:

(I've just found ticket #3075, which asks to fix this. It'd be easier to mark that other ticket as a duplicate of this one than the other way around.)

Why not move this patch to #3075? I don't have any questions about the patch and we close #3075.

Yes, associate that patch with the other ticket and close it. Thanks.

You can then close this ticket as well (mark it as "invalid" or something), because I don't see a way other than my original patch (the one introducing [main:empty-cmdline]). If anybody has another idea, she/he can open a new ticket. There's no reason to keep this ticket around. For the record: I see nothing wrong in my original patch (specifically: I don't see the extra map as a hack; on the contrary).

(BTW, patch "v2" turned out to have a major bug but I won't bother to explain or fix it: that patch was mainly meant for "brainstorming", not as a real solution.)

[andrew_b]

Replying to mooffie:

Replying to andrew_b:

Replying to mooffie:

(I've just found ticket #3075, which asks to fix this. It'd be easier to mark that other ticket as a duplicate of this one than the other way around.)

Why not move this patch to #3075? I don't have any questions about the patch and we close #3075.

Yes, associate that patch with the other ticket and close it. Thanks.

Applied as [4a0f265d95643b4f0f84c388b12acf1f406b63a6]. Thanks.

[mooffie]

@Andrew, lets try to salvage this ticket. I'll try to summarize the situation:

The patch which introduces a new keymap, [main:empty-cmdline], wasn't accepted. Andrew, you wrote:

I think we don't need extra section [main:empty-cmdline]. These commands
should be routed to the active panel.

This seems logically impossible to me, and here's why: Our purpose (the users' purpose) is to trigger certain commands when the commandline is empty. In other words, we (that is, the users) need a way to flag key bindings as being effective only when the commandline is empty. A quick reminder:

• #2375: A user wants backspace, on empty commandline, to chdir up.
• #3591: A user wants backslash, on empty commandline, to switch panels.
• #3551: A user wants ampersand, on empty commandline, to trigger some command (comment 10).

We don't currently have any means to flag such bindings. If we don't want to hardcode the keys (as is the situation now), then there currently isn't a solution. The [failed] patch at comment:9, which does not use an extra keymap, works by hardcoding: it works only for punctuation marks (and not for <backspace> or <left>, for example).

Any solution needs either a flagging element, or a hardcoding element.

By introducing a keymap, [main:empty-cmdline], we solve the problem because, inherently, a keymap is a flag: a binding is flagged by being there.

I don't see a problem of elegance in introducing this keymap. On the contrary. It's not a hack:

For 20 years we've had this one mode, "the command-line is empty", establishing itself. There's no reason not to name this legitimate boy.

---

Now, let's look at the opposite solution: ticket #2375. That ticket was solved by introducing a new command that checks for an empty commandline and if so acts as the old command. We had CK_CdParent before, and that ticket introduced CK_CdParentSmart (it was named differently 6 years ago, though). The "Smart" part is in checking for an empty commandline. The problem with this solution is that it's hardcoding: the user can only use commands for which a "Smart" parallel version has been coded in C.

---

This was a long post (I hope you're not annoyed; I know you're busy here in gazillion of other things), so let me sum it up by asking:

1. Could you explain the problem you see, if at all, in having this extra keymap?
2. "Any solution needs either a flagging element, or a hardcoding element." Do you agree with this statement?
3. The new keymap names an established mode, and thereby enables us to refactor the code. Isn't this elegance?

[andrew_b]

I think, the 'empty commandline mode' is default mode. 90% actions are made with empty command line. Probably, we need (if we really need) section [main:non-empty-cmdline].

Actions Select, Unselect and SelectInvert? from [main] are working with empty command line only.
But those actions from [panel] are working regardless of whether command line empty or no. Probably, there is a chance to join them. If no, then we have to add an extra keybing section. But it will be the 3rd (3rd, Carl!) binding of some actions.

[mooffie]

I've created a ​sequence diagram of the filemanager's keymaps handling, for whoever wants to study it:

(The lookups are painted in pink.)

(It was created using ​this script.)

[mooffie]

Replying to andrew_b:

Actions Select, Unselect and SelectInvert? from [main] are working with empty command line only.

Not so: the actions themselves work with either non-empty or empty command-line. Just like all the other actions (except the rogue CK_CdParentSmart I mentioned), they don't care about the status of the command line. That's how things should be.

What we're dealing with is key bindings, not actions. It's certain key bindings that we want to limit to "the command line is empty" condition.

As for Select / Unselect / SelectInvert? from [main]: they're actually bound to the + - * keys of the keypad, not to the ASCII keys, so they work for non-empty command lines as well (BTW, we can also invoke them using the menu; so, again, we see that the actions themselves don't care about the status of the command line). The + - * ASCII keys, OTOH, aren't bound using any keymap: they're hardcoded in MSG_KEY of midnight_callback() (and the purpose of this ticket is to get rid of this hardcoding; the [main] keymap is fine: there's nothing to fix in it).

I think, the 'empty commandline mode' is default mode. 90% actions are made with empty command line.

It's not that they're "made with empty command line". They don't care about the command-line: they work with either empty or non-empty one.

Probably, we need (if we really need) section [main:non-empty-cmdline].

IIUC, you're proposing to make 90% of bindings only work when the command-line is empty? It'd be wrong: the user should have the freedom, for example, to hit F6 (rename) or change the "Listing mode" even when there's something typed on the command-line.

We don't have a problem with those 90% of bindings. What we want to deal with is the 10% (probably more like 4%) that want an empty command-line. This minority is the the exception, and a keymap dedicated for them seems like a straightforward solution.

[andrew_b]

Ticket #3947 has been marked as a duplicate of this ticket.