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.

Sign up for GitHub

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

Jump to bottom

autocmd: RecordingEnter, RecordingLeave ("macro" events) #15407

Closed
wants to merge 6 commits into from
Closed

autocmd: RecordingEnter, RecordingLeave ("macro" events) #15407

wants to merge 6 commits into from

Conversation

vigoux
Copy link
Member

@vigoux vigoux commented Aug 17, 2021 •
edited
Loading

Add some autocmds to determine when the user is recording macros.

TODO:

  • Add RecordingStart and RecordingStop autocmds
  • Add reg_recorded(), to query what was the latest recorded register
  • Provide a default mapping using these features

Fixes partially #15404, cc @clason @justinmk

@vigoux vigoux force-pushed the recording-autocmd branch from a2b4766 to 43c7fae Compare August 17, 2021 14:37
gpanders
gpanders reviewed Aug 17, 2021
View reviewed changes
runtime/doc/eval.txt Outdated Show resolved Hide resolved
@vigoux vigoux force-pushed the recording-autocmd branch 3 times, most recently from 18e0e19 to e2f2a0f Compare August 17, 2021 15:43
zeertzjq
zeertzjq reviewed Aug 18, 2021
View reviewed changes
src/nvim/normal.c Outdated Show resolved Hide resolved
@vigoux vigoux force-pushed the recording-autocmd branch from e2f2a0f to 18ac947 Compare August 19, 2021 11:59
@justinmk justinmk changed the title Add Recording autocmds #15404 autocmd: RecordingEnter, RecordingLeave ("macro" start/stop event) #15404 Aug 22, 2021
@justinmk justinmk changed the title autocmd: RecordingEnter, RecordingLeave ("macro" start/stop event) #15404 autocmd: RecordingEnter, RecordingLeave ("macro" events) #15404 Aug 22, 2021
justinmk
justinmk reviewed Aug 22, 2021
View reviewed changes
runtime/doc/autocmd.txt Outdated Show resolved Hide resolved
justinmk
justinmk reviewed Aug 22, 2021
View reviewed changes
src/nvim/normal.c Outdated Show resolved Hide resolved
@vigoux vigoux force-pushed the recording-autocmd branch from 18ac947 to 271904e Compare August 23, 2021 09:07
@vigoux vigoux requested a review from justinmk August 23, 2021 09:07
@justinmk justinmk mentioned this pull request Aug 23, 2021
Closed
@zeertzjq
Copy link
Member

zeertzjq commented Aug 26, 2021 •
edited
Loading

Maybe ex should be removed from 'belloff'?

@vigoux
Copy link
Member Author

vigoux commented Aug 26, 2021

Oh indeed this should be removed

justinmk
justinmk reviewed Aug 26, 2021
View reviewed changes
src/nvim/normal.c Outdated Show resolved Hide resolved
justinmk
justinmk reviewed Aug 26, 2021
View reviewed changes
src/nvim/auevents.lua Outdated Show resolved Hide resolved
justinmk
justinmk reviewed Aug 26, 2021
View reviewed changes
runtime/doc/autocmd.txt Outdated Show resolved Hide resolved
justinmk
justinmk reviewed Aug 26, 2021
View reviewed changes
runtime/doc/autocmd.txt Outdated Show resolved Hide resolved
justinmk
justinmk reviewed Aug 26, 2021
View reviewed changes
runtime/doc/eval.txt Outdated Show resolved Hide resolved
@justinmk justinmk changed the title autocmd: RecordingEnter, RecordingLeave ("macro" events) #15404 autocmd: RecordingEnter, RecordingLeave ("macro" events) Aug 26, 2021
@ggandor ggandor mentioned this pull request Aug 29, 2021
Open
@vigoux vigoux force-pushed the recording-autocmd branch from c07e425 to 6f8c532 Compare August 30, 2021 13:06
@clason clason added core Nvim core functionality or code defaults Nvim defaults for options, colorscheme/theme, autocmds/events, etc. enhancement feature request events events, autocommands labels Sep 10, 2021
vigoux added 2 commits October 1, 2021 09:36
@vigoux
feat(autocmd): add Recording autocmds
208b805
@vigoux
feat(eval): add reg_recorded()
eda025f 
This function is used the get the last recorded register.
@vigoux vigoux force-pushed the recording-autocmd branch from 6f8c532 to eda025f Compare October 1, 2021 07:37
vigoux added 4 commits October 1, 2021 09:40
@vigoux
docs(reg_recorded): add links to relevant docs
47a8e4d
@vigoux
docs(Recording): update docs to match implementation
473f27c
@vigoux
style(Recording): rename handler to match suggestions
e95b735
@vigoux
fix(RecordingLeave): send autocommand earlier
b287186 
This makes the autocommand fire just before setting reg_recorded to
reg_recording, this way we clearly show that we are actually just before
actually quitting the recording mode.
@clason clason requested a review from justinmk October 1, 2021 07:52
@vigoux
Copy link
Member Author

vigoux commented Oct 1, 2021

I should have addressed all the comments, so I think I just need @justinmk 's approval on this and we'll be good to go !

@zeertzjq
Copy link
Member

zeertzjq commented Oct 3, 2021 •
edited
Loading

There are still some references to the old Q in docs:

  • eval.txt line 6884
  • vim_diff.txt line 358
  • repeat.txt line 106 (this should be changed to gQ)
  • index.txt line 342
  • quickref.txt line 999
  • options.txt line 953 (I have mentioned this above)

There are also a lot of oldtests failing because they use Q.

@AckslD
Copy link
Contributor

AckslD commented Oct 6, 2021

@vigoux Thanks for working on this! A few days ago I had an idea of doing something which would need such events so really happy to see this here :)

@zeertzjq
Copy link
Member

zeertzjq commented Oct 18, 2021 •
edited
Loading

There are still some references to the old Q in docs:

* `eval.txt` line 6884

* `vim_diff.txt` line 358

* `repeat.txt` line 106 (this should be changed to `gQ`)

* `index.txt` line 342

* `quickref.txt` line 999

* `options.txt` line 953 (I have mentioned this above)

There are also a lot of oldtests failing because they use Q.

I think the oldtest failures can be solved by mapping Q to gQ in setup.vim.

@AckslD
Copy link
Contributor

AckslD commented Dec 13, 2021

@vigoux @zeertzjq Is there anyway I can help out with getting this ready? For example updating the docs and tests as you pointed out @zeertzjq or something else. Really looking forward to this so I'm happy to help out if possible :)

@zeertzjq zeertzjq added the needs:response waiting for reply from the author label Dec 13, 2021
@gpanders
Copy link
Member

@AckslD @vigoux is pretty busy right now with other commitments, so feel free to take over this PR. At this point it should just need test and doc updates.

Be sure to throw a reference/link to this PR when you create your new one.

@vigoux
Copy link
Member Author

vigoux commented Dec 14, 2021

Hey sorry for the delay, you can of course take this PR over, and you can drop me an email whenever you need any help / guidance, and I'll try my best to help you out.

Thanks for taking that over BTW !

@AckslD
Copy link
Contributor

AckslD commented Dec 16, 2021

Hi @gpanders @vigoux, no problem, I'll try to take a look at this and update test and docs and get back to you when done or if I have questions :)

zeertzjq
zeertzjq reviewed Dec 16, 2021
View reviewed changes
runtime/doc/autocmd.txt
The pattern is the current file name, and
|reg_recording()| is the current register that
is used.
*RecordinLeave*
Copy link
Member

Choose a reason for hiding this comment

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

I just found a typo here.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed in #16684

@AckslD AckslD mentioned this pull request Dec 16, 2021
Merged
@AckslD
Copy link
Contributor

AckslD commented Dec 16, 2021

I just opened #16684 where I so far fixed the things in the docs that @zeertzjq pointed out.

@zeertzjq regarding the old tests, are there ever new tests added there? Just wondering if it would be fine to just map Q to gQ as you suggested.

@gpanders
Copy link
Member

Moved to #16684.

@gpanders gpanders closed this Dec 16, 2021
@seandewar seandewar removed the needs:response waiting for reply from the author label Dec 16, 2021
@dundargoc dundargoc removed the request for review from justinmk October 3, 2022 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zeertzjq zeertzjq zeertzjq left review comments

@AckslD AckslD AckslD left review comments

@gpanders gpanders gpanders left review comments

@justinmk justinmk justinmk left review comments

Assignees
No one assigned
Labels
core Nvim core functionality or code defaults Nvim defaults for options, colorscheme/theme, autocmds/events, etc. enhancement feature request events events, autocommands
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@vigoux @zeertzjq @AckslD @gpanders @justinmk @clason @seandewar