Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

Improvement preserve cursor position #24

Merged

Conversation

camilledejoye
Copy link
Contributor

This PR must be merged after #23 since it uses the hook system.

The goal of this MR is to respond to the issue #7
The general idea is that the only way to preserve the position of the cursor is relative to the argument under it.
I leverage the container dictrionary to remember on which argument and which column (relative to the begening of the argument and not the line) the cursor was on.
Then after an operation the post hook event calculate the new position of the cursor a place it.

A first step to make the plugin extendable.
This allow to add logic on a per filetype basis.

This solution allow a user to extend the behavior if the plugin does not
provide a hook for the given filetype.
But if there is already a hook then the user can not create it's own.

One solution could be to find every functions defined in the namespace
`argwrap#hooks#user#<ANYTHING>#post_wrap`, I think airline provide a
feature like this.
Those hooks would need to be sorted in order to have a predictible
behavior, also it might be interesting to provide a way for each hook to
update the range, container and arguments.
There might be nothing to do because I think Vim's lists and dictionaries
are passed by reference.
This solve the issue FooSoft#19
I put it there as an example, it might be better in its own repository.
This way a user will be able to choose to use it or not, which will
allow them to be able to provide their own implementation.

This attempt only deal with methods.
Functions are not part of the PSR-2.
Closures should always have the opening brace on the same line as the
closing parenthesis, this extension is not a CS fixer and therefore as
no reason to deal with them.
It is still not possible to override an existing hook but with this
system users can create new hooks really simply without having to update
any configuration since they will be autodetected.

The only missing thins is the possibility to disable hook, therefore it
is the responsibility of each hook to provide a way to the user to use
it or not.
The hooks were executed in the same order regarding of if they were pre
or post hooks.
This is actually not a correct behavior, let's assume we want to create
a hook responsible for keeping the position of the cursor between
operations:
The pre hook will have to extract the current position of the cursor and
therefore should be the first one to be executed, let's say we gave it a
priority 0.
The post hook will have to reposition the cursor after the
transformation and therefore should be the last one to be executed.
That's why we need to reverse the order of the post hooks.
The settings were initialize in an autoload which is not a good
practice.
So I initialize them in the plugin directly, this way there are
initialize only once when the plugin is loaded and they can be access
anywhere without having to worry about what the default value.
The idea was to be able to keep the cursor on the same position than
before an operation.
To solve this issue I first extract the position of the cursor, this
position is given relative to the argument list. It contains the number
of the argument and a column number relative to the start of the
argument.
This way it is possible to position the cursor later on and I hope it
should be pretty easy for any extension to adapt it if needed since it's
provided in the container dictionary.
@camilledejoye
Copy link
Contributor Author

@FooSoft is possible to have an update on the status of the two pending merge requests ?

@FooSoft
Copy link
Owner

FooSoft commented Jun 27, 2020

@elythyr sorry about that! Notifications must have gotten buried in my email box -_-
Both PRs look great, thank you for your contribution : )

@FooSoft FooSoft merged commit f3d122d into FooSoft:master Jun 27, 2020
@camilledejoye
Copy link
Contributor Author

No worries and my pleasure: I love this plugin :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants