Skip to content

Conversation

@jyscao
Copy link
Collaborator

@jyscao jyscao commented Nov 15, 2019

  • provide API for adding reasonable keymappings in Nop Mode as exceptions
  • added keymappings will only work in the explicitly excepted modes
  • added 3 sets of exceptions (2 for Buffer, 1 for File) as examples

* provide API for adding reasonable keymappings in Nop Mode as exceptions
* added keymappings will only work in the explicitly excepted modes
* added 3 sets of exceptions (2 for Buffer, 1 for File) as examples
@jyscao
Copy link
Collaborator Author

jyscao commented Nov 15, 2019

A few more notes:

  • this PR fixes issue Can not reload file list when no files match #226 which is caused by line 597 in autoload/ctrlspace/window.vim: call s:modes.Nop.Enable(), even though File mode will also be enabled still. Removing said line will allow mappings like r to work in the situation specified in issue Can not reload file list when no files match #226, but I do not think that'd be best since it's there to restrict unpredictable effects. Therefore, the workaround devised is to explicitly add whatever keys one wishes to be functional as exceptions in NOP mode

  • to do so, just add a call to s:AddExceptionMapping (like the 3 that already exist) inside of the ctrlspace#keys#nop#ExceptionsInit function in file autoload/ctrlspace/keys/nop.vim. The arguments in order are: 1) the excepted action, 2) the mode that must be enabled (in addition to NOP mode) for the excepted action to work, and 3) the allowed keys. In other words, s:AddExceptionMapping has the same function signature as ctrlspace#keys#AddMapping

  • I also added switching & moving tabs as excepted actions that can be executed in Nop + Buffer mode. Without these, a lone unlisted buffer (the runtime log from a plugin I use opens such a buffer for instance) causes Buffer mode to also enter Nop mode, thus preventing tab switching and moving

  • other potential excepted actions can be things like ctrlspace#keys#buffer#NewTabLabel in Buffer mode, tab moving and tab switching in File mode (these two actions are currently only excepted for Buffer mode). Unless they're requested, I did not see the need to include an overabundance of such actions for their own sake; but should they be needed/wanted in the future, it's now quite easy to add them

@bodograumann
Copy link
Contributor

Thanks for taking on this issue! These changes indeed fix my problem in #226.

Now my thoughts on the code:

“Exception” is a pretty generic term. Something more specific would probably be better. Cannot think of something more fitting of the top of my head though.

Whether you find a better term or not, I think you should add some comments in the code to better explain what this new mechanism does.

I see you have also added something to the help. First I thought it did not work, because I did not see it. Turns out it is only visible when you press ? in NOP mode.
My suggestion would be to instead use the same logic as in ctrlspace#keys#nop#_ExecException and only show the help if we are currently in the applicable mode.

What do you think?

* generate help descriptions only in the applicable combined modes
* ex. help for 'r' key only visible in FILE + NOP mode
* instead of "Exception", use "Dbmdex" for DouBle MoDe EXception
@jyscao
Copy link
Collaborator Author

jyscao commented Nov 17, 2019

@bodograumann thanks for the detailed feedback, I updated the PR to account for your suggestions

“Exception” is a pretty generic term

I had the same hesitation with the name to be frank, but the generic meaning of the word indeed described the feature in this case. So now I've opted to use an unsightly but memorable acronym instead, with a header that spells out the meaning behind it.

My suggestion would be to instead use the same logic as in ctrlspace#keys#nop#_ExecException and only show the help if we are currently in the applicable mode.

This is an excellent idea. I had thought it'd be best to have this as well, but disappointingly for whatever reason completely missed that I could just do a simple s:modes[<Mode>].Enabled check like I did before. And instead, I was thinking along the lines of creating a new set of APIs in keys.vim to be able to temporarily add new mappings then removing them afterwards, which ofc would've made things significantly more complicated. So thanks for the timely tip.

And lastly, it's my pleasure to help with this, since for one this is an awesome plugin, and two, this also solves the problem I'd occasionally run into with BUFFER+NOP modes. Aside from the logging buffer case that I briefly mentioned, :tab help foo will also opens a nonlisted buffer in a new tab, which limits CtrlSpace's Buffer mode from functioning all that well.

@bodograumann
Copy link
Contributor

Very nice. Just one more thing:
You write “Define NOP + excepted mappings in the following Initiator”. Could it be, that you mean “accepted mappings”? Because these mappings are accepted in NOP mode. The two words are pronounced almost exactly the same...

@jyscao
Copy link
Collaborator Author

jyscao commented Nov 18, 2019

I added a clarification for that section

@cometsong cometsong merged commit 9d52446 into vim-ctrlspace:master Jan 2, 2020
Konfekt added a commit to Konfekt/vim-ctrlspace that referenced this pull request Jan 4, 2020
* upstream/master:
  Add Feature: Exception Keymaps in NOP Mode (vim-ctrlspace#245)
  encourage custom set-up of Go engine (vim-ctrlspace#250)
  Add wrap-around for moving/copying buffers/tabs (vim-ctrlspace#242)
Konfekt pushed a commit to Konfekt/vim-ctrlspace that referenced this pull request Jan 4, 2020
* Add Feature: Exception Keymaps in NOP Mode

* provide API for adding reasonable keymappings in Nop Mode as exceptions
* added keymappings will only work in the explicitly excepted modes
* added 3 sets of exceptions (2 for Buffer, 1 for File) as examples

* Minor tidying up + more concise help description

* Shorten help description for excepted keymappings

* Improve: help descriptions for NOP + <Mode>

* generate help descriptions only in the applicable combined modes
* ex. help for 'r' key only visible in FILE + NOP mode

* Change feature name (less ambiguity)

* instead of "Exception", use "Dbmdex" for DouBle MoDe EXception

* Add: code comments for new feature

* Elaborate on code comment
@jyscao jyscao deleted the nop_workaround branch June 23, 2020 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants