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

Add motion between frames #1853

Merged
merged 3 commits into from
Nov 12, 2020
Merged

Add motion between frames #1853

merged 3 commits into from
Nov 12, 2020

Conversation

RiMillo
Copy link
Contributor

@RiMillo RiMillo commented Nov 10, 2020

I find the provided motions (by section, environment,...) very handy but I think it could be useful to have one by frame to make the editing of beamer documents easier.

I took the liberty to write a first version. I just took the environment-related functions and modified them a little bit. It seems to work on simple examples but I did not fully test it and there definitely is room to improvement.

I propose r-based shortcuts (so, for instance, ]r or [R): I would have gone with f, but it is already taken for instance in tpope/vim-unimpaired.

No documentation is given.

Hope it could be useful.

(I'm very sorry if I did not follow the contributions rules, please have mercy of a Git newbie :-) )

@lervag
Copy link
Owner

lervag commented Nov 10, 2020

I find the provided motions (by section, environment,...) very handy but I think it could be useful to have one by frame to make the editing of beamer documents easier.

I took the liberty to write a first version. I just took the environment-related functions and modified them a little bit. It seems to work on simple examples but I did not fully test it and there definitely is room to improvement.

Thank you; I agree this is useful!

I propose r-based shortcuts (so, for instance, ]r or [R): I would have gone with f, but it is already taken for instance in tpope/vim-unimpaired.

I don't mind the r-based shortcuts. Do you know if b conflicts with anything; it could perhaps be better as a menomic to Beamer?

No documentation is given.

Do you mean you don't want to propose the docs? I can probably add it myself if that's the case.

(I'm very sorry if I did not follow the contributions rules, please have mercy of a Git newbie :-) )

Don't worry, I think the PR smells of high quality.

Before I merge, I would like to add a couple of tests in the test/tests/test-motion directory. Do you want to have a go at it, or do you want me to do it?

@RiMillo
Copy link
Contributor Author

RiMillo commented Nov 10, 2020

I don't mind the r-based shortcuts. Do you know if b conflicts with anything; it could perhaps be better as a menomic to Beamer?

Unfortunately, in the aforementioned plugin, b is used as well (for things like :bnext). And e, too. In fact, considering vimtex and vim-unimpaired of all the letters of beamer and frame, only r is available, I think.

No documentation is given.

Yes, I meant that I didn't write any addition to the docs. But I can come up with something with the same method as the patch (aka copy-and-paste :-) ).

Before I merge, I would like to add a couple of tests in the test/tests/test-motion directory. Do you want to have a go at it, or do you want me to do it?

I don't mind helping out, but I am not familiar with this sort of things and I think won't be able to do it all by myself, sorry. In fact, are you asking to provide a MWE?

@lervag
Copy link
Owner

lervag commented Nov 11, 2020

Unfortunately, in the aforementioned plugin, b is used as well (for things like :bnext). And e, too. In fact, considering vimtex and vim-unimpaired of all the letters of beamer and frame, only r is available, I think.

Ok, no problem. Let's keep r.

No documentation is given.

Yes, I meant that I didn't write any addition to the docs. But I can come up with something with the same method as the patch (aka copy-and-paste :-) ).

Sounds good!

Before I merge, I would like to add a couple of tests in the test/tests/test-motion directory. Do you want to have a go at it, or do you want me to do it?

I don't mind helping out, but I am not familiar with this sort of things and I think won't be able to do it all by myself, sorry. In fact, are you asking to provide a MWE?

I think what would be good is a test similar to test-math-motions, i.e. two files like the following that lives in test/tests/test-motions:

Simply adding test-beamer-motions.tex and test-beamer-motions.vim with the proper content suffices for adding the tests. Again, I think the "copy paste" method should work quite well, but let me know if you still don't see how to do this. Its not so much work, so I could also do it myself.

@RiMillo
Copy link
Contributor Author

RiMillo commented Nov 11, 2020

I pushed two commits, one with the docs update and one with the test.

About the latter, it seems to work on my version of vim+vimtex, however I'm stuck with vim 8.0 and I'm currently using g:vimtex_version_check=0 and the version of vimtex that I use dates back to a couple of months ago. So, I think you should test it on your side as well.
Thanks again!

@lervag lervag merged commit a592a77 into lervag:master Nov 12, 2020
@lervag
Copy link
Owner

lervag commented Nov 12, 2020

Thanks, this looks very good! I've tested and I fixed a minor annoyance (which was also present in some of the other motions) where the jump list was not properly set (i.e. doing 2]r then <c-o> did not go back to where we started).

@RiMillo
Copy link
Contributor Author

RiMillo commented Nov 25, 2020

It seems that I forgot to add the new motion in the README.md. I'll do a new PR. Sorry to bother again.

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.

None yet

2 participants