Open shawnrice opened 10 years ago
As mentioned by email, there's a change I'd like to suggest:
AB_DATA/bundler/includes/bindings/{lang}/{name}.ext
I believe this to be better for all languages (less copy-pasta):
$bindings_dir = '/path/to/bindings/php/';
require_once($bindings_dir . 'Terminal-Notifier.php');
require_once($bindings_dir . 'CocoaDialog.php');
is preferable to:
$bindings_dir = '/path/to/bindings/';
require_once($bindings_dir . 'Terminal-Notifier/Terminal-Notifier.php');
require_once($bindings_dir . 'CocoaDialog/CocoaDialog.php');
There are also two Python-specific issues:
terminalnotifier.py
, terminal_notifier.py
or TerminalNotifier.py
(the first is preferable).bindings/
php/
…
python/
bindings/
__init__.py
terminalnotifier.py
…
ruby/
…
That would significantly reduce the complexity of any Python implementation. There are ways to work around the above issues, but I don't think "so it works like the PHP one" is a compelling reason to make the Python (or any other) bundler more complex than it otherwise need be, especially as it would require code that borders on "magic".
In general, I think divvying things up by language is better than by function: the latter encourages inter-bundler consistency, and therefore implicitly a lowest-common-denominator or even hacky approach. Organising things by language encourages, imo, doing what's best by, and most natural in, that language.
This has already come up a couple of times with minor issues. All the bundlers are called AlfredBundler.ext
, which is not a common naming scheme for all languages, and the Python wrapper cannot be named alfred.bundler.py
, like the others, while the Ruby one cannot be named bundler.rb
, as it might then clash with a very popular library (I think. Ruby isn't my strong point.)
As I've said elsewhere, I think consistency with the language trumps consistency with other bundler implementations. Nobody enjoys using, say, a Ruby library that is basically a transliteration of some other language and its idioms.
I'm sold. 851e457aff1c74ebfa9741c3ad00bc65b43bfcaa reorganizes the directory structure to follow your proposal, @deanishe. I didn't write the __init__.py
file, so that's on the todo list.
@deanishe and I were talking about the name bindings
; I know that I'm the one who suggested it, but they aren't exactly bindings. They are actually wrappers
. The initial problem is that we already have something called "wrappers" in the bundler, and calling them both "wrappers" would indubitably confuse people.
So, here is the proposal: we recast the current "wrappers" as "interfaces" and rename the "bindings" to "wrappers". While the current wrappers are more wrappers than interfaces, it helps things out a bit.
We could also find a different yet appropriate name for the current bindings. Or we could do nothing at all.
@Ritashugisha: Thoughts?
WRT what we currently call wrappers: Yeah, they're wrappers from an implementation point of view, but interfaces from the user's point of view (the wrapping should hopefully be transparent). I think that's a fair reason to rename them interfaces. Or possibly something else. Bundlets? Bundlings?
Daves?
I don't mind renaming the bindings to wrappers. The only thing that it really effects would be the documentation.
Okay, so wrappers
become interfaces
and bindings
become wrappers
.
However, other actual wrappers (fork.sh
, update_wrapper.sh
) should keep the wrapper
designation.
Current files to change wrapper
to interface
:
bundler/AlfredBundler.php: * this wrapper will continue to work with the bundler API for the remainder of
bundler/AlfredBundler.py:Simply include this ``bundler.py`` file (from the Alfred Bundler's ``wrappers``
bundler/AlfredBundler.py: 'update-wrapper.sh')
bundler/AlfredBundler.py: '{}/wrappers/alfred.bundler.misc.sh'.format(BUNDLER_VERSION))
bundler/AlfredBundler.py:# install them if necessary. This is actually the bash wrapper, not
bundler/AlfredBundler.py:HELPER_PATH = os.path.join(HELPER_DIR, 'bundlerwrapper.sh')
bundler/AlfredBundler.py: # Call bash wrapper with specified arguments
bundler/AlfredBundler.rb: command = "'" + @data + "/bundler/wrappers/alfred.bundler.misc.sh' '#{name}' '#{version}' '#{type}' '#{json}'"
bundler/AlfredBundler.sh:# Specific wrapper for the load function
bundler/benchmark.php:require_once( 'wrappers/alfred.bundler.php' );
bundler/benchmark.sh:. wrappers/alfred.bundler.sh
bundler/meta/defaults/schema.json.example:# __load() function included with the wrapper.
bundler/meta/update.sh:# and the minor versions are integers. The wrapper for each major version will
bundler/wrappers/alfred.bundler.misc.sh: # This is a wrapper function for the AlfredBundler::load function to make
bundler/wrappers/alfred.bundler.php: * this wrapper will continue to work with the bundler API for the remainder of
bundler/wrappers/alfred.bundler.php: // The 'AlfredBundler' class is a small wrapper of a class. All the calls
bundler/wrappers/alfred.bundler.php: // Call the wrapper to update itself, this will fork the process to make
bundler/wrappers/alfred.bundler.php: * need to bundle any dependencies with your workflows. Other wrappers for
bundler/wrappers/alfred.bundler.php: * wrapper that allows any other language to load assets via a Bash script.
bundler/wrappers/alfred.bundler.rb: # Bundler URLs have to be hard coded in the wrapper
bundler/wrappers/alfred.bundler.sh: # This is a wrapper function for the AlfredBundler::load function to make
bundler/wrappers/bundler.py:Simply include this ``bundler.py`` file (from the Alfred Bundler's ``wrappers``
bundler/wrappers/bundler.py: '{}/bundler/wrappers/alfred.bundler.misc.sh'.format(
bundler/wrappers/bundler.py:# install them if necessary. This is actually the bash wrapper, not
bundler/wrappers/bundler.py:HELPER_PATH = os.path.join(HELPER_DIR, 'bundlerwrapper.sh')
bundler/wrappers/bundler.py:_log = logging.getLogger('bundler.wrapper')
bundler/wrappers/bundler.py: """Check if bundler bash wrapper is installed and install it if not.
bundler/wrappers/bundler.py: if not os.path.exists(HELPER_PATH): # Install bash misc wrapper
bundler/wrappers/bundler.py: # Install bash wrapper from GitHub
tests/php/AlfredBundler.php:require_once( __DIR__ . '/../../bundler/wrappers/alfred.bundler.php' );
Daves?
+1
More seriously, I do kinda like "bundlets".
It implies both that they're kinda bundlers, but not full bundlers.
Technically, I suppose "bundler placeholder" or "bundler stub" is the proper designation, but that sucks.
Come on. Let's call them "bundlets". It has character!
I'm cool with that.
So wrappers
stay as wrappers
and bindings
turn to bundlets
?
Bindings to wrappers and wrappers to bundlets.
Okay. I made the name change. Things might break, although, at least between the Bash, PHP, and Ruby bundlers, as well as some other supporting scripts I had to change only one instance of wrapper
to bundlets
in the code.
But, that means I might have totally borked the Python bundler. I'm assigning you to fix that @deanishe.
No problem. My bundler has test coverage…
And mine has non-committed emoji support...
I think you allocated your time better. :cry:
Yes, I think I probably did :wink:
I flushed out a good few errors adding unit tests, including broken updating. I think I'm close to 100% coverage.
I'm sure you had more fun, though …
Surprisingly, read through loads of pages on unicode specs isn't that much fun.
I've implemented the name changes in the Python version.
I've been pondering how this should work in Python for a while now.
My current thinking is that AlfredBundler.py
(the bundler) should have a trivial _notify()
method for its own use ("installing stuff…", "updating stuff…"), and bundler.py
(bundlet and user API) should provide the proper API for the wrappers.
If AlfredBundler.py
doesn't import wrappers
, wrappers
is free to import AlfredBundler.py
, which provides the paths to the utilities the wrapper classes need. That way, it's possible for wrappers
to also implement helper functions that instantiate CocoaDialog
or Pashua
objects directly.
The PHP bundler takes care of this by providing a simple method:
$b = new AlfredBundler;
$cd = $b->wrapper( 'cocoadialog' );
That method should just require_once
the wrapper and return a CD wrapper object that has been instantiated with the the bundler's method to load cocoadialog.
So, basically it just gives them the simple object they would need to use CD through the wrapper.
Not sure if that could be applied to to Python...
Sure it can, but you can't have both AlfredBundler.py
importing wrappers
and wrappers
importing AlfredBundler.py
. It's a circular import. bundler.py
(the bundlet) can import both, however.
It's a question of whether it's better to do wrappers.utility('cocoaDialog')
or bundler.wrapper('cocoaDialog')
.
The former doesn't require the wrappers to always be imported. But I suppose if the bundler is supposed to have a full-featured notify()
function that uses the wrapper (#32), that doesn't matter, and the latter is the way to go.
That makes sense. The latter is a decent way to go especially because importing the wrapper modules shouldn't take much time (they're small, ultimately).
Btw, updated checklist at the top as something nested.
@Ritashugisha , @deanishe , are these implemented in the Python bundler?
We need to provide an easy way to load bindings for users, however the language works best.
For PHP (and I think Ruby), we just have to look to see if the wrapper exists and then require it.
wrapper
tobundlet
binding
towrapper
So, for PHP, the implementation creates a method that is invoked by:
Then, we have the cocoadialog bindings loaded and ready to use. Currently, it just looks for a file in
AB_DATA/bundler/includes/wrappers/{name}/{name}.php
. Hence, the folder and the inner names need to be the same. We should normalize these to match the asset name in the defaults. Hence, we need to haveTerminal-Notifier
andCocoaDialog
.