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

Would you like quite a lot of changes? #22

Open
wants to merge 106 commits into
base: master
Choose a base branch
from

Conversation

andrew-sayers--frontier-forums

Hi,

I've used BabelExt as a framework for building forum extensions (a bit like RES, but on a much smaller scale). I've added quite a bit of functionality as part of that work. Would you be interested in taking some or all of it upstream? Major changes include:

  • PhantomJS build system. I know it sounds crazy, but if you think about it, PhantomJS is a cross-platform technology with great support for JSON and XML config files, that can programatically push new versions to every major extension portal
  • Browser preferences (or settings, or whatever your browser calls them). I take the point that making your own HTML config page is better for complex stuff, but my users asked for it so I've added it :)
  • BabelExt.utils.dispatch(). Dispatches to different callbacks depending on the URL, parameters and page elements, and passes in stored values and preferences. For certain types of extension (i.e. what I wrote), this makes the actual extension much easier to read and write

I realise there's a lot of stuff here so I don't expect you to accept the request wholesale. But please let me know what if anything would interest you in principle, and what if anything I can do to put it in a format you'd be happy to pull.

makelinks.bat had several issues (bashisms with #!/bin/sh, incorrect handling
of .user.js files etc.)

Rather than keep a second version in sync with the .bat file that seems
better-maintained, this commit rewrites the whole thing to read the .bat and
mimic its behaviour.
Note: this replaces makelinks.sh
addons.mozilla.org will reject packages uploaded using the git version.
Confuses the Chrome store
Disallowed by the Chrome store
Modern versions of Opera use Blink, so are similar enouh to Chrome not to need
special treatment.

Older versions of Opera had very primitive extension support.

Removing support for older versions of Opera allows us to support more
functionality with less work.
…nd ...remove()

The existing behaviour is very confusing to a newbie, as console.log() doesn't
usually go anywhere useful, so you just see a function return without doing
what you expect.

Throwing an error is more useful for get(), because callbacks are really
required there.

Inserting a no-op callback is more useful for set() and remove(), where you
usually don't care.
This library allows us to style Chrome settings like native settings
The latter is supported by all browsers now old Opera is gone
Added ‘env’ to build scripts so OS X can find phantom.js, should work
across all platforms.
@ghost
Copy link

ghost commented Jun 22, 2015

Do you think this will ever be merged?

@honestbleeps
Copy link
Owner

@voltagex - have you had the chance to try it out and/or review? I have been insanely busy and while I'm excited to look at these changes, my free time has sadly been very very limited. This is a monstrously large PR and so I believe it really needs some peer review that I've not been able to commit yet.

I'm definitely open to merging it after some conversation and some clarity.

@ghost
Copy link

ghost commented Jun 22, 2015 via email

@honestbleeps
Copy link
Owner

I'd sincerely like to get some or all of it merged, it's just so monumental it's hard to review. A split would help immensely so we could at least test/merge bits of it piecemeal and decide if anything is/isn't right. For example, I'm not initially sure if I agree with the inclusion of the CSS files, etc - but I haven't had the opportunity to fully review.

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

Successfully merging this pull request may close these issues.

3 participants