Skip to content

Conversation

jgollenz
Copy link
Contributor

@jgollenz jgollenz commented Apr 24, 2022

This PR adds support for mappings in other modes than normal-mode, as discussed in #249 .

@kristijanhusak fyi I'm still working on the part concerning your vim.keymap.set example ;)

if type(keys) == 'string' then
keys = { keys }
local map = {}
if #mappings[category][name] == 0 then -- we are in a multi-mode mapping
Copy link
Member

@kristijanhusak kristijanhusak Apr 24, 2022

Choose a reason for hiding this comment

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

I think we should use this check:

type(mappings[category][name]) == 'table' and not vim.tbl_islist(mappings[category][name])

Because we have situation where we have a mapping that is a table list, but is actually a single normal mapping with an additional parameter, like this one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm not mistaken that example should work regardless, because it should still go into the else branch (since the length operator would return non-zero) 🤔 But you are absolutely right, my check is ugly and I just didn't know how to do it properly 😁 Thanks a lot, will add the change :)

@jgollenz jgollenz force-pushed the feature/multi-mode-mappings branch from 7703088 to 122149b Compare April 24, 2022 20:59
@kristijanhusak kristijanhusak marked this pull request as ready for review April 24, 2022 21:28
Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

This looks good now. You will cover the other options also in this PR?

@jgollenz
Copy link
Contributor Author

You will cover the other options also in this PR?

Yes, should find time for it this week

@jgollenz
Copy link
Contributor Author

jgollenz commented May 1, 2022

some_multi_mapping = {
    x = 'org_mappings.previous_visible_heading_visual', -- different function if needed. If not, use the same value as for normal mapping
    o = {'org_mappings.previous_visible_heading_operator', true }, -- pass down the arguments to a function if needed.
}

@kristijanhusak we could make the first mapping equal to x = { 'org_mappings.previous_visible_heading_visual' }, demanding it to be a table. That would make it more consistent with how the basic mapping works and remove one additional way to do things.

@kristijanhusak
Copy link
Member

@jgollenz yeah, do that. More consistent is better.

@jgollenz jgollenz force-pushed the feature/multi-mode-mappings branch from 3d3d8a9 to 0a85f4a Compare May 20, 2022 19:30
@jgollenz jgollenz force-pushed the feature/multi-mode-mappings branch from 0a85f4a to f1fe5f2 Compare May 20, 2022 19:31
@jgollenz
Copy link
Contributor Author

@kristijanhusak sorry, it took a while. Should be working now

Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@kristijanhusak kristijanhusak merged commit 5aed4b7 into nvim-orgmode:master May 25, 2022
@jgollenz jgollenz deleted the feature/multi-mode-mappings branch June 7, 2022 06:15
gzagatti pushed a commit to gzagatti/orgmode that referenced this pull request Oct 19, 2022
SlayerOfTheBad pushed a commit to SlayerOfTheBad/orgmode that referenced this pull request Aug 16, 2024
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.

2 participants