Initial Plugin Architecture patch
Matt Tolton
Attached is the initial plugin architecture patch that I'd like to
submit. File 0001 is code written by Bjorn to handle plugin menu
items. Patches 0002 and 0003 are supporting files that I needed for
the plugin gui stuff. RBSplitView is released under the MIT license.
CTGradient is in the public domain. Ironically, I don't think that
either of these classes would be necessary if we were aiming for 10.5,
rather than 10.4 compatibility.
I have also attached a tarball which contains the xcode project for a
basic plugin that displays a file browser as well as a buffer list.
Known limitations/issues with this patch:
- The plugin panels don't work in full screen mode -- the drawer just
isn't shown. I am ok with this, and am not really interested in ever
getting the drawer working in full screen mode. The plan is to
eventually get rid of the drawer completely and use the more "cool"
side panel look that seems to be in now.
- The settings for the plugin panels aren't remembered (i.e. size,
order). This is on the todo list.
- I realize that evaluateExpressionCocoa is not an acceptable name.
:) I'm assuming that it will be changed to evaluateExpression as soon
as we change any current uses of evaluateExpression to use the new
method. At that point the old method will be removed, and the new one
renamed.
I probably left a few things off of that list that I can't remember right now.
Thanks,
Matt
björn
Matt,
Thank you for the (huge!) patch. I will have to look at this closely
before I start merging and so far I've only had a chance to compile
and test the example plugin (it works :-). Before I'll consider
merging I'd like you to add an #ifdef around all the plugin code
(including the menu stuff I sent you) so that it is possible to
compile MacVim without any plugin code at all being included. You can
add the define in MacVim.h.
Just quickly: I noticed that the keyboard focus is a bit weird.
Sometimes when I type the keyboard input will go both to the plugin
drawer and the editor window. There should always be at most one view
which is "key" so there is obviously something strange going on there.
I'll give more feedback when I get a proper chance to look at the code.
Thanks again,
Björn
Matt Tolton
> merging I'd like you to add an #ifdef around all the plugin code
> (including the menu stuff I sent you) so that it is possible to
> compile MacVim without any plugin code at all being included. You can
> add the define in MacVim.h.
Ok... I will leave the #ifdef off of the evaluateExpressionCocoa
stuff, though...since that is general.
>
> Just quickly: I noticed that the keyboard focus is a bit weird.
> Sometimes when I type the keyboard input will go both to the plugin
> drawer and the editor window. There should always be at most one view
> which is "key" so there is obviously something strange going on there.
Yes, I'm pretty sure that this is something funky to do with drawers.
I was kind of thinking that it would be up to the plugin developer to
ensure that their view gives up focus, but maybe that's the wrong
approach. Perhaps there is something I could do to make sure that
none of the events are passed out of the drawer...perhaps by making
the plugin view respond to any event, so it doesn't get passed on to
the window?
Matt
björn
>
>> and test the example plugin (it works :-). Before I'll consider
>> merging I'd like you to add an #ifdef around all the plugin code
>> (including the menu stuff I sent you) so that it is possible to
>> compile MacVim without any plugin code at all being included. You can
>> add the define in MacVim.h.
>
> Ok... I will leave the #ifdef off of the evaluateExpressionCocoa
> stuff, though...since that is general.
Ok, thanks.
>> Just quickly: I noticed that the keyboard focus is a bit weird.
>> Sometimes when I type the keyboard input will go both to the plugin
>> drawer and the editor window. There should always be at most one view
>> which is "key" so there is obviously something strange going on there.
>
> Yes, I'm pretty sure that this is something funky to do with drawers.
> I was kind of thinking that it would be up to the plugin developer to
> ensure that their view gives up focus, but maybe that's the wrong
> approach. Perhaps there is something I could do to make sure that
> none of the events are passed out of the drawer...perhaps by making
> the plugin view respond to any event, so it doesn't get passed on to
> the window?
Please read up on the Cocoa docs so that you know what's going on and
so that you can fix it. Unfortunately I don't know exactly what's
going on here so I haven't got any advice. Somehow, if the drawer
receives the input then it must not let it "slip" to the text view.
(It should work the same as if you had e.g. two text views in one
window.)
Björn
björn
I've looked through your plugin patch a bit more now and it looks very
good to me. Some parts I skimmed through (like the plugin gui stuff)
but the modifications to existing files is quite minimal so I am
happy.
I made notes while looking through the code, here they are (some
issues are so minor that they're hardly worth mentioning, but
anyway...):
- MMPlugInManager missing dealloc
- Remove NSLog() calls (e.g. in loadAllPlugIns)
- Ensure isInitialized is set _after_ all initializations (instanceMediator is
init'ed after isInitialized is set at the moment)
- Add short description of class at the top of .m file (add yourself as author
of the file too)
In the example plugin:
- Don't hardcode path to home dir in example plugin (confused me for a second)
- Let validateMenuItem: return NO if there is no key plugin instance
The only real objection I have towards merging at this point in time
are the issues I brought up in another post in this thread.
Björn
Matt Tolton
> - MMPlugInManager missing dealloc
Oops.
> - Remove NSLog() calls (e.g. in loadAllPlugIns)
You don't think that these should stay in? I think that they're
useful...I intentionally left them in.
> - Ensure isInitialized is set _after_ all initializations (instanceMediator is
> init'ed after isInitialized is set at the moment)
Hmm. This was also intentional. I intentionally put it afterward
because I felt that the initialization of the plugin instances was
separate from initializing vim. Can you elaborate on what exactly
isInitialized is for?
> - Add short description of class at the top of .m file (add yourself as author
> of the file too)
Ok.
> In the example plugin:
> - Don't hardcode path to home dir in example plugin (confused me for a second)
> - Let validateMenuItem: return NO if there is no key plugin instance
Oh, yeah...oops. The example plugin really is not intended to be
robust at the moment, but I'll fix those. :)
Matt
Matt Tolton
> so that you can fix it. Unfortunately I don't know exactly what's
> going on here so I haven't got any advice. Somehow, if the drawer
> receives the input then it must not let it "slip" to the text view.
> (It should work the same as if you had e.g. two text views in one
> window.)
Bjorn,
I'm having trouble finding the documentation I remember reading, but
from what I recall it said that, when a drawer has focus, actions not
responded to by the drawer's responder chain were then sent down the
window's responder chain.
I'm really not sure what to do about this. Especially since the
drawer is a temporary thing until we get side panels, I'd like to
ignore it for now. Once we replace it with a side panel, this
shouldn't be an issue.
Thanks,
Matt
björn
>
>> - Remove NSLog() calls (e.g. in loadAllPlugIns)
> You don't think that these should stay in? I think that they're
> useful...I intentionally left them in.
When you use 'mvim' those logs pop up in Terminal which is very
annoying so I don't think they should be there. As a rule I only
NSLog things that are exceptional somehow (and then I try to fix the
problem causing the log).
>> - Ensure isInitialized is set _after_ all initializations (instanceMediator is
>> init'ed after isInitialized is set at the moment)
> Hmm. This was also intentional. I intentionally put it afterward
> because I felt that the initialization of the plugin instances was
> separate from initializing vim. Can you elaborate on what exactly
> isInitialized is for?
Well, it's no big deal, but the point of that variable is to indicate
that everything in the class has been initialized.
I just though of another thing: update Credits.rtf (i.e. the "About"
dialog) with information about the new classes that you are using
(RBSplitView, ...).
Björn
björn
>
> I'm having trouble finding the documentation I remember reading, but
> from what I recall it said that, when a drawer has focus, actions not
> responded to by the drawer's responder chain were then sent down the
> window's responder chain.
>
> I'm really not sure what to do about this. Especially since the
> drawer is a temporary thing until we get side panels, I'd like to
> ignore it for now. Once we replace it with a side panel, this
> shouldn't be an issue.
Hmmm...I still find this a bit weird, but maybe it is something the
plugin writer has to take care of.
Can you somehow fix (i.e. hack) the example plugin so that it behaves?
That is: the outline view should only "jump" to the letter you type
if you have first clicked on the view so that it has focus. If you
click on the text view and type, the outline view should not do
anything.
Björn
Matt Tolton
> That is: the outline view should only "jump" to the letter you type
> if you have first clicked on the view so that it has focus. If you
> click on the text view and type, the outline view should not do
> anything.
Hrm, this is already how it works for me...can you give me steps to
reproduce this behavior?
Matt
Matt Tolton
> annoying so I don't think they should be there. As a rule I only
> NSLog things that are exceptional somehow (and then I try to fix the
> problem causing the log).
Ahh...ok. I'll remove the ones that indicate success, but leave the
failure ones in.
Matt
Stefan Eletzhofer
is there a git repository I could pull from? Applying the patches
manually via git-apply
gave me failed hunks on the last patch (0004-*) of the patches sent
by you earlier. Or is there
already a branch?
I'd like to test this out to get started writing plugins, I'd like to
use the PyObjC bridge
to create a plugin using python.
Also, sorry if this is obvious to you, where do the plugins need to be
stored? I assume
somewhere in "~/Library/Applucation Support"?
Thanks,
Stefan.
> <0001-Fixed-outstanding-issues-with-plugin-architecture-pa.patch>
--
Stefan Eletzhofer
InQuant GmbH
Bahnhofstraße 11
D-88214 Ravensburg
http://www.inquant.de
+49 (0) 751 35 44 112
+49 (0) 751 35 44 115 (FAX)
Geschäftsführer: Lutz Mende
HRB 552769, St.Nr.: 77025/22069, USt.-IdNr.: DE245179196
Matt Tolton
> is there a git repository I could pull from? Applying the patches
> manually via git-apply
> gave me failed hunks on the last patch (0004-*) of the patches sent
No, I don't have a public git repo at the moment. It looks like Bjorn
will be applying them soon, so it's probably not worth setting one up.
The patches probably failed because Bjorn pushed some modifications
to the repo after I sent the patches out.
I have rebased my local branch to the main repo, and have attached an
updated patch series to this email. These should work for you.
Matt
Matt Tolton
> stored? I assume
> somewhere in "~/Library/Applucation Support"?
~/Library/Application Support/MacVim/PlugIns/
björn
>
> The attached patch should address all of the outstanding issues. It
> is to be applied on top of the patches I previously sent.
Matt,
The final patch looks great...thank you very much for putting the
effort in and adding plugin support. I am in the process of merging
now but am a bit tired so it will probably be in the public repo
tomorrow.
I figured out exactly what's going on with "leaking key presses" from
the drawer. If you select the drawer and press a key, the drawer will
swallow that keypress _if_ it handles it, otherwise it is passed on to
the window and the text view gets it. So, to see this you can e.g.
set the browser plugin to point to a folder with no files starting
with the letter "i". Then press "i" and you'll see Vim going into
insert mode. Even if the folder contains a file beginning with "i",
if you just press "i" enough times it will eventually "spill" into the
text view. Now that I understand what's going on I don't really think
its a big deal, but it is a bit of a nuisance. Not sure what can be
done about it.
Anyway, excellent work...now I'm looking forward to some nice plugins... ;-)
Björn
Stefan Eletzhofer
sorry to bug you.
I did apply your patches, recompiled, build the plugin example
and put it into ~/Library/Application Support/MacVim. I used the
patches you sent today, thanks.
I started vim using the terminal to see stderr/stdout.
But there's no plugin visible :)
Is there a hidden option/magic dance/whatever to summon the
plugin drawer?
FWIW, the PluginView NIB is in the newly built bundle -- so it DOES
compile it in.
Any hints?
--
Matt Tolton
> and put it into ~/Library/Application Support/MacVim.
Wrong directory...it should be:
~/Library/Application Support/MacVim/PlugIns/
björn
I've pushed your patch to the public repo. I did some rearrangement
to get a nice commit history so please go over it quickly to make sure
I didn't introduce any errors (I've looked it over too, of course).
I have two feature requests for you:
1. Add plugin versioning. Just something simple like
"<major>.<minor>" where the plugin manager only loads the plugin if
its major version matches (which would change if the interface
changes). You can use CFBundleVersion inside each plugin's Info.plist
for the version number (starting with e.g. 1.0). We're already doing
something similar in the "Integration pane" of the prefs panel with
the input manager, if you want to take a look at that. I'm sure you
know what I mean, but let me know if I'm being vague.
2. Add plugin preference pane. It would be nice to be able to
enable/disable plugins by ticking a check-box (it's ok if you have to
restart for this to take effect).
I'd say the former is important and the second is not. We need a
versioning scheme before people start writing plugins or chaos will
ensue.
Björn
Matt Tolton
> to get a nice commit history so please go over it quickly to make sure
> I didn't introduce any errors (I've looked it over too, of course).
Great! I'll look at it.
> I have two feature requests for you:
>
> 1. Add plugin versioning. Just something simple like
> "<major>.<minor>" where the plugin manager only loads the plugin if
> its major version matches (which would change if the interface
> changes). You can use CFBundleVersion inside each plugin's Info.plist
> for the version number (starting with e.g. 1.0). We're already doing
> something similar in the "Integration pane" of the prefs panel with
> the input manager, if you want to take a look at that. I'm sure you
> know what I mean, but let me know if I'm being vague.
Ok, I'll do this soon.
> 2. Add plugin preference pane. It would be nice to be able to
> enable/disable plugins by ticking a check-box (it's ok if you have to
> restart for this to take effect).
I will probably wait on this until it feels more necessary, if that's
ok. There are some other things that I'd like to do that seem more
important (like implementing autocmd notifications).
Matt
björn
>
>> 2. Add plugin preference pane. It would be nice to be able to
>> enable/disable plugins by ticking a check-box (it's ok if you have to
>> restart for this to take effect).
>
> I will probably wait on this until it feels more necessary, if that's
> ok. There are some other things that I'd like to do that seem more
> important (like implementing autocmd notifications).
Of course. I don't think this is very important...it's easy to just
move the plugin from its folder so that it does not get loaded.
Auto-command support seem by far more important (and useful).
Björn
Matt Tolton
While I'm thinking about it, I have a question:
Where should I put menu items that belong to the plugin system, but
not to a plugin. For example, I need to add something like "Toggle
Left Drawer". Should I just add these to the plugin menu when the
plugin system starts up, and add a separator after them? It seems
like it might be nice to have that one specifically in the Window
menu, but isn't that essentially owned by the vim process?
Matt
Nico Weber
> I've pushed your patch to the public repo. I did some rearrangement
> to get a nice commit history so please go over it quickly to make sure
> I didn't introduce any errors (I've looked it over too, of course).
from the patch this looks as if there will be a menu item inserted
called "Plug-Ins" when there's at least one plug-in. Many vim plugins
(NERDCommenter, Rails, VCSCommand) put stuff into a global "Plugin"
menu. So if you have both MacVim plugins and those plugins, you'll
have both "Plugin" and "Plug-Ins" in your menu bar. This is weird.
Nico
Matt Tolton
> called "Plug-Ins" when there's at least one plug-in. Many vim plugins
> (NERDCommenter, Rails, VCSCommand) put stuff into a global "Plugin"
> menu. So if you have both MacVim plugins and those plugins, you'll
> have both "Plugin" and "Plug-Ins" in your menu bar. This is weird.
Yep. Have a suggestion as to an alternative?
Matt
Nico Weber
The MacVim plugins menu could be called "Plugin" as well, and contain
vim plugin menu entries at the bottom, after a separator. I.e.:
Plugins:
General MacVim plugin stuff
separator
list of MacVim plugins
separator
menu entries of vim plugins
Nico
Matt Tolton
> General MacVim plugin stuff
> separator
> list of MacVim plugins
> separator
> menu entries of vim plugins
I assume "list of MacVim plugins" just means "menu entries added by
MacVim plugins"? Is that correct?
Nico Weber
Yes.
George Harker
One small thing - I couldn't tell how to reopen the drawer if I close
it. I may just be stupidly missing it
Cheers
George
Matt Tolton
> One small thing - I couldn't tell how to reopen the drawer if I close
> it. I may just be stupidly missing it
Right now there isn't a way to reopen it. I will be adding this sometime soon.
Matt