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

Expand target selection to other Ag commands #25

Closed
amerlyq opened this issue Nov 29, 2015 · 8 comments
Closed

Expand target selection to other Ag commands #25

amerlyq opened this issue Nov 29, 2015 · 8 comments

Comments

@amerlyq
Copy link
Collaborator

amerlyq commented Nov 29, 2015

You introduced auto-select function ag#GetArgs.
It has sense to expand its usage to other ag#... functions, to introduce more useful <Plug>(ag-... mappings.

@amerlyq
Copy link
Collaborator Author

amerlyq commented Nov 30, 2015

I propose instead of calling GetArgs inside each of function, it would be more clean to make them more independent and short, and do all arguments binding inside of commands definitions.
Then defiinition of <Plug> mappings will look like :<C-u>Ag*...<CR>.
Another variant -- use GetArgs as special binding function, which can be called like

:<C-u>call ag#autoargs('ag#AgBuffer') 

then you can keep <Plug> mappings to call functions directly.

@amerlyq
Copy link
Collaborator Author

amerlyq commented Dec 4, 2015

To make architecture more flexible and finally allow to implement #23, #26 clearly, I grouped functions into categories by type of argument for ag they provide. And then distributed them over separate files, (with changed signatures). In result we can create any previously existed function (and some more new) simply by combining those API together. Moreover, introducing new arguments now is as simple as adding new function, because all glue was separated out. After adding tests on standard functions, I will smooth somewhat fluffy API introduced in dc21da1.

It's somewhat big commit, because I couldn't think of incremental way to introduce splitting and changes in architecture. This work took much of my free time to refine and discard dozens of possible implementations, until I decided on this way. Therefore if you have objections I simply fork out:) Because I really need that changes, even if you agrees with maintainer at rking#124 about non-intrusive contributing.
(Edit) I will split that commit in smaller pieced and rewrite history though. But not until think out some logical way to do so.

@albfan
Copy link
Owner

albfan commented Dec 4, 2015

Just a few notes: I like to be on the edge and clearly ag.vim codebase isn't robust. So all your changes are right for me. Just see the far you get the hard you will be on contribute ag.vim upstream. You are creating a real fork rather than a contribute fork.

The question is: Apart for upgrade your own workbench, do you want to share these with the community? In that case you need to honor the recognition rking repo achieve and accept worst codebase and other programmers vision to contribute the community and see your work on other machines.

For me the option is to take both approachs. Do whatever nasty things you want here on master, but keep an eye on upstream to share as much as you can to bring back something to community.

@amerlyq
Copy link
Collaborator Author

amerlyq commented Dec 4, 2015

Speaking about priority -- its edge first sharing second. Not too distant though.
Honoring old codebase is good, but it the exact reason why that repo became stale.
Being lazy I would liked to not make much changes, if I could:)
Therefore if I could pull flames up at Gitter on those changes and vote, it would be nice.
After all, even if there exists maintainer, it's community to decide if them want those changes in upstream. If those features are worth it.

@amerlyq
Copy link
Collaborator Author

amerlyq commented Dec 8, 2015

I think, I will not merge long-lived feature_args_derive in master after all.
After working two weeks on it, I got some comprehension about bounds of its composition and overall goals.
I will cover that branch with tests, then transpose linear history to parallel features, and delete origin.
Hope it will reduce problems, if someone will agree to merge my work in upstream.

@albfan
Copy link
Owner

albfan commented Dec 9, 2015

Good move. Lot to say? Speak it slowly.

@albfan
Copy link
Owner

albfan commented Dec 11, 2015

Might we close this issue by now? We can reopen later if needed

@amerlyq
Copy link
Collaborator Author

amerlyq commented Dec 11, 2015

Alright, it has sense. Work was done, though not merged yet.
I will simply shot up new issues for each part of it's content.

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

No branches or pull requests

2 participants