Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: add composable mappings, e.g. use a separate key to jump between snippet tabstops #48

Merged
merged 1 commit into from
Dec 10, 2021

Conversation

smjonas
Copy link
Collaborator

@smjonas smjonas commented Dec 7, 2021

  • add "actions" users can use to compose their own mappings
  • update readme

@smjonas
Copy link
Collaborator Author

smjonas commented Dec 7, 2021

@mathjiajia could you please test this PR and confirm that everything works as expected? :)

@smjonas
Copy link
Collaborator Author

smjonas commented Dec 7, 2021

I noticed an annoying behaviour in the mappings, will fix it

@smjonas smjonas marked this pull request as draft December 7, 2021 17:45
@smjonas smjonas marked this pull request as ready for review December 7, 2021 17:53
@smjonas
Copy link
Collaborator Author

smjonas commented Dec 7, 2021

I'll wait for @mathjiajia before merging, then I will squash

@mathjiajia
Copy link
Contributor

mathjiajia commented Dec 7, 2021

Dear @smjonas, thanks so much.
I tested with this PR (with the 'alternative' mapping) a bit and it works well.

BTW, if you want to add some explanation in README, below may be better?
Since for those who does not Tab to jump forward, they will not use S-Tab to jump back.
How do you think?

["<Tab>"] = cmp.mapping(
          function(fallback)
            cmp_ultisnips_mappings.expand_or_jump_forwards(fallback)
            -- Alternatively, if you prefer a separate key for jumping between snippet tabstops:
            -- cmp_ultisnips_mappings.expand(fallback)
          end,
          {
            "i", "s",
            -- add this line when using cmp-cmdline:
            -- "c",
          }
        ),
["<S-Tab>"] = cmp.mapping(
          function(fallback)
            cmp_ultisnips_mappings.jump_backwards(fallback)
            -- Alternatively, if you prefer a separate key for jumping between snippet tabstops:
            -- if cmp.visible() then
            --   cmp.select_prev_item()
            -- else
            --   fallback()
            -- end
          end,
          {
            "i", "s",
            -- add this line when using cmp-cmdline:
            -- "c",
          }
        ),

@smjonas
Copy link
Collaborator Author

smjonas commented Dec 8, 2021

Thanks for the testing it! I just had another idea how to handle this problem: we could
make the mappings "composable": what I mean is we can provide the basic functions expand, jump-forwards, jump_backwards, select_next, select_prev. We can still keep the defaults as they are now in the readme. Then users can define which of them they want to use in a specific order by passing a table. And we can still provide a default example in the readme. What do you think?

@quangnguyen30192
Copy link
Owner

Sounds good to me 💪

@smjonas
Copy link
Collaborator Author

smjonas commented Dec 8, 2021

Unrelated: do you guys still see =UltiSnips#ExpandSnippet() in the command line when pressing tab? That was supposed to be fixed already but I just saw it again...

@quangnguyen30192
Copy link
Owner

it still persists yeah

@smjonas smjonas marked this pull request as draft December 8, 2021 09:27
@smjonas
Copy link
Collaborator Author

smjonas commented Dec 8, 2021

But it only happens when nothing is selected, right? Like at the beginning of a line?

Edit: it seems to occur when cmp calls fallback(). I am not sure if it can be fixed. At least it happens less often now.

Another thing I noticed: are you able to jump when in select mode in a mapping? I thought it should work with UltiSnipsRemoveSelectModeMappings.

@mathjiajia
Copy link
Contributor

mathjiajia commented Dec 8, 2021

Thanks to both of you.
The "composable" mapping idea is great.

BTW, =UltiSnips#ExpandSnippet() disappears in my computer since this PR #42.
Maybe in my usage, fallback() occurs rarely.

@smjonas
Copy link
Collaborator Author

smjonas commented Dec 9, 2021

Btw I didnt't forget about this PR, just thought it was fun to work on treesitter integration for a bit ;) (see #50)

@smjonas smjonas force-pushed the add_expand_mapping branch 2 times, most recently from 63620f3 to 0414f2b Compare December 10, 2021 09:58
@smjonas smjonas marked this pull request as ready for review December 10, 2021 10:01
@smjonas smjonas force-pushed the add_expand_mapping branch 4 times, most recently from 688474c to b074963 Compare December 10, 2021 10:16
@smjonas smjonas changed the title Add expand mapping Feat: add composable mappings, e.g. use a separate key to jump between snippet tabstops Dec 10, 2021
Copy link
Owner

@quangnguyen30192 quangnguyen30192 left a comment

Choose a reason for hiding this comment

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

Looks pretty good

@smjonas
Copy link
Collaborator Author

smjonas commented Dec 10, 2021

Thanks

@smjonas smjonas merged commit e563ede into quangnguyen30192:main Dec 10, 2021
@smjonas smjonas deleted the add_expand_mapping branch December 10, 2021 11:14
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