Differential D12524

Changes in several files because, by default, F2 renames and F9 opens a terminal window
ClosedPublic
Actions

Authored by asensi on Apr 25 2018, 6:38 PM.

Details

Reviewers
nmel
martinkostolny
yurchor
Group Reviewers
Krusader
Commits
R167:53a5e29ccc44: Changes in several files because, by default, F2 renames and F9 opens a…
Test Plan

Using F2 and F9 in several cases, including reassigning those keys.

Diff Detail

Repository
R167 Krusader
Lint
Lint Skipped
Unit
Unit Tests Skipped
asensi requested review of this revision.Apr 25 2018, 6:38 PM
asensi created this revision.
nmel accepted this revision.Apr 25 2018, 8:51 PM
nmel added a subscriber: nmel.

Thanks Toni. This patch fixes Edit and Tools menu on clean setups.
Unfortunately, it has no effect if user opened the app at least once as ~/.local/share/kxmlgui5/krusader/krusaderui.rc is never updated later.

This revision is now accepted and ready to land.Apr 25 2018, 8:51 PM
martinkostolny accepted this revision.Apr 25 2018, 9:06 PM
martinkostolny added a subscriber: martinkostolny.

Thanks Toni!

asensi edited the test plan for this revision. (Show Details)Apr 25 2018, 9:29 PM
nmel added a comment.Apr 26 2018, 3:36 AM

including reassigning those keys.

I have these keys reassigned for a long time. It works fine regardless of this patch. The modified shortcuts are displayed in the menu correctly.

(independent of the patch) IMO, what a bad name scheme is picked for the actions. Names shouldn't contain default values. A better naming could be FN_Rename if one would like to denote these belong to the functional keys bar.

asensi added a comment.Apr 26 2018, 7:30 AM

(independent of the patch) IMO, what a bad name scheme is picked for the actions. Names shouldn't contain default values. A better naming could be` FN_Rename` if one would like to denote these belong to the functional keys bar.

Yes, I think the same.

Additionally, I have made some related changes, including one in a docbook file.

asensi updated this revision to Diff 33124.Apr 26 2018, 7:32 AM
asensi added a subscriber: yurchor.

Some related changes were made, including one in a docbook file.

Restricted Application added a subscriber: kde-doc-english. · View Herald TranscriptApr 26 2018, 7:32 AM
This revision now requires changes to proceed.Apr 26 2018, 7:38 AM
nmel added a comment.Apr 26 2018, 7:43 AM
In D12524#254073, @yurchor wrote:

The change in docbook is not relevant anymore.

https://cgit.kde.org/krusader.git/commit/?id=1d9988330eac92f1701f11131ec24f95c8d711eb

Yuri, this is a different file: menu-commands.docbook.

yurchor accepted this revision.Apr 26 2018, 7:43 AM
In D12524#254080, @nmel wrote:
In D12524#254073, @yurchor wrote:

The change in docbook is not relevant anymore.

https://cgit.kde.org/krusader.git/commit/?id=1d9988330eac92f1701f11131ec24f95c8d711eb

Yuri, this is a different file: menu-commands.docbook.

Oops... Sorry.

This revision is now accepted and ready to land.Apr 26 2018, 7:43 AM
nmel added a comment.Apr 26 2018, 7:50 AM

Nice catch, Toni. Please see my inline comment.

Please update the commit message accordingly. Thanks!

krusader/total_commander.keymap
3

Do I recall correctly that Totcmd doesn't have any of these shortcuts: Rename, Terminal, Multirename? I don't remember exactly because I probably had defined them a long time ago as I use F2 and F9 without hassle. :)

asensi updated this revision to Diff 33128.Apr 26 2018, 8:27 AM
asensi retitled this revision from Changes in krusaderui.rc because F2 renames and F9 opens a terminal window to Changes in several files because normally F2 renames and F9 opens a terminal window.

The commit message was updated accordingly.

asensi added a comment.Apr 26 2018, 8:28 AM
In D12524#254087, @nmel wrote:

Nice catch, Toni. [...]
[Do I recall correctly that Totcmd doesn't have any of these shortcuts: Rename, Terminal, Multirename? I don't remember exactly because I probably had defined them a long time ago as I use F2 and F9 without hassle. :) ]

Total Commander has those internal commands and those shortcuts by default:

cm_ExecuteDOS       [None that I have found]
cm_RenameOnly       Shift+F6
cm_MultiRenameFiles Ctrl+M

and you can assign a shortcut to an internal command, I suppose that that's what was meant :-)

martinkostolny accepted this revision.Apr 26 2018, 9:36 PM

Accepting the update, thanks!

nmel accepted this revision.Apr 27 2018, 5:25 AM
In D12524#254117, @asensi wrote:
In D12524#254087, @nmel wrote:

Nice catch, Toni. [...]
[Do I recall correctly that Totcmd doesn't have any of these shortcuts: Rename, Terminal, Multirename? I don't remember exactly because I probably had defined them a long time ago as I use F2 and F9 without hassle. :) ]

Total Commander has those internal commands and those shortcuts by default:

cm_ExecuteDOS       [None that I have found]
cm_RenameOnly       Shift+F6
cm_MultiRenameFiles Ctrl+M

and you can assign a shortcut to an internal command, I suppose that that's what was meant :-)

I confirmed today that Shift+F6 and Ctrl+M work in Total Commander as F2 and Shift+F2 in Krusader. I see Ctrl+M is assigned for media action in keymap... :/

I think it's fine to merge it as is because the patch is clearly an improvement and the keymap needs a more fundamental update.

asensi retitled this revision from Changes in several files because normally F2 renames and F9 opens a terminal window to Changes in several files because, by default, F2 renames and F9 opens a terminal window.Apr 27 2018, 11:43 AM
Closed by commit R167:53a5e29ccc44: Changes in several files because, by default, F2 renames and F9 opens a… (authored by asensi). · Explain WhyApr 27 2018, 11:44 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 33128

View Options

doc/menu-commands.docbook

Loading...
View Options

krusader/krusaderui.rc

Loading...
View Options

krusader/total_commander.keymap

Loading...