394779 - Add commandline invocation to storage-explorer

Status: RESOLVED FIXED

(Other Applications Graveyard :: mozStorage Explorer, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Add a commandline flag (like -inspector for DOMi) that will open the storage explorer window when opening the host application.

Possible flags: -storageexplorer or -sqlexplorer or -storage

Comment 1

[u278084]

Attachment: a beta patch. should work when put into .xpi

The attached patch should work if you package it into the xpi. I believe some more work has to be done to get it working with the tree. So It is not complete yet, AFAIK anyways. This is my first time doing a command line handler, and it shows.

BTW, the flag is -storage

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 279871 [details] [diff] [review]
a beta patch. should work when put into .xpi

Just a drive by review. Overall it looks good.

>Index: components/cmdline-handler.js

>+
>+const storage-explorer_CID =
>+    Components.ID("{111c9704-603b-4e55-9806-d008179ee9cc}");
>+const storage-explorer_contract_id = 
>+    "@mozilla.org/commandlinehandler/general-startup;1?type=storage";
>+
>+const window-watcher = "@mozilla.org/embedcomp/window-watcher;1";
>+const categorymanager = "@mozilla.org/categorymanager;1";

Use all CAPS for CID and contractid global consts and use underscores instead of dashes.

>+
>+/* Resource : http://developer.mozilla.org/en/docs/Chrome:_Command_Line */
>+var StorageCommandlineHandler = {
>+    QueryInterface : function(iid) {
>+        if (iid.equals(Components.interfaces.nsISupports) ||
>+            iid.equals(Components.interfaces.nsIFactory) ||
>+            iid.equals(Components.interfaces.nsICommandLineHandler))
>+            return this;
>+        throw Components.results.NS_ERROR_NO_INTERFACE;
>+    },
>+
>+    handle : function(cmdline) {
>+        if (cmdline.handleFlag("storage", false)) {
>+            var ww = Components.classes[window-watcher]
>+                .getService(Components.interfaces.nsIWindowWatcher);
>+            ww.openWindow(null, 
>+                          "chrome://storage-explorer/content/storage-explorer.xul",
>+                          "_blank",
>+                          "chrome,dialog=no,all",
>+                          null);
>+        }
>+    },
>+
>+    createInstance : function(outer, iid) {
>+        if (outer != null) {
>+            throw Components.results.NS_ERROR_NO_AGGREGATION;
>+        }
>+        return StorageCommandlineHandler.QueryInterface(iid);
>+    },
>+
>+    lockFactory : function(lock) {},
>+
>+    helpInfo : " -storage              Opens up Storage Explorer",
>+};

I'm not sure about -storage as the flag. It seems to general.

-sqlexplorer is the best I can come up with now. Maybe someone else can think of a better flag.

>+
>+var StorageCommandlineModule = {
>+    QueryInterface : function(iid) {
>+        if (iid.equals(Components.interfaces.nsIModule) ||
>+            iid.equals(Components.interfaces.nsISupports))
>+            return this;
>+
>+        throw Components.results.NS_ERROR_NO_INTERFACE;
>+    },
>+
>+    getClassObject : function(compMgr, cid, iid) {
>+        if (cid.equals(storage-explorer_CID) == true)
>+            return StorageCommandlineHandler.QueryInterface(iid);
>+
>+        throw Components.results.NS_ERROR_NOT_REGISTERED;
>+    },
>+
>+    registerSelf : function(compMgr, fileSpec, location, type) {
>+        compMgr.QueryInterface(Components.interfaces.nsIComponentRegistrar);
>+
>+        compMgr.registerFactoryLocation(storage-explorer_CID,
>+                                    "StorageCommandlineHandler",
>+                                    storage-explorer_contract_id,
>+                                    fileSpec,
>+                                    location,
>+                                    type);
>+
>+        var catMan = Components.classes[categorymanager]
>+                        .getService(Components.interfaces.nsICategoryManager);
>+        catMan.addCategoryEntry("command-line-handler",
>+                                "m-storage",
>+                                storage-explorer_contract_id, true, true);
>+    },
>+
>+    unregisterSelf : function(compMgr, location, type) {
>+        compMgr.QueryInterface(Components.interfaces.nsIComponentRegistrar);
>+        compMgr.unregisterFactoryLocation(storage-explorer_CID, location);
>+
>+        var catMan = Components.classes[categorymanager]
>+                        .getService(Components.interfaces.nsICategoryManager);
>+        catMan.deleteCategoryEntry("command-line-handler", "m-storage");
>+    },
>+
>+    canUnload : function(compMgr) {
>+        return true;
>+    }
>+}
>+
>+function NSGetModule(comMgr, fileSpec) {
>+    return StorageCommandlineModule;
>+}
>+

We could use the new XPCOMUtils for this part but then it wouldn't work in Fx2

Comment 3

[Shawn Wilsher :sdwilsh]

(In reply to comment #2)
> We could use the new XPCOMUtils for this part but then it wouldn't work in Fx2
It's OK.  I don't really plan on this targetting Fx2 to be honest.  I'd much rather target Fx3 with this, and I really want to use XPCOMUtils (makes the code so much cleaner).

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 279871 [details] [diff] [review]
a beta patch. should work when put into .xpi

Cesar - treat the drive by review as a real review.

If Shawn is not looking to support Fx2, then I do think you should us XPCOMUtils.jsm to implement the XPCOM stuff.

Comment 5

[u278084]

Attachment: redone using xpcomutils and -sqlexplorer handler

(In reply to comment #2)
> I'm not sure about -storage as the flag. It seems to general.
> 
> -sqlexplorer is the best I can come up with now. Maybe someone else can think
> of a better flag.

-sqlexplorer it is. -storage was just simple to write and easiest to remember.

(In reply to comment #3)
> It's OK.  I don't really plan on this targetting Fx2 to be honest.  I'd much
> rather target Fx3 with this, and I really want to use XPCOMUtils (makes the
> code so much cleaner).

Now that it is done, it is considerably cleaner. Thanks for the heads up, I didn't know something like this existed.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 280426 [details] [diff] [review]
redone using xpcomutils and -sqlexplorer handler


>+StorageCommandlineHandler.prototype = {
>+  classDescription : "storage-explorer",
>+  classID : Components.ID("{b07a8d1c-8397-4022-a5c6-72cc0731610c}"),
>+  contractID : "@mozilla.org/commandlinehandler/general-startup;1?type=storage",
>+
>+  QueryInterface : XPCOMUtils.generateQI(
>+    [Ci.nsICommandLineHandler]),
>+
>+  _xpcom_categories : [{
>+    category : "command-line-handler",
>+    entry : "m-storage-explorer",

entry is usually "m-" + type, so can we change the contractID type to "sqlexplorer" (to match the flag) and then entry : "m-sqlexplorer"?

r=mfinkle with those changes

Comment 7

[Reed Loden [:reed]]

Comment on attachment 280426 [details] [diff] [review]
redone using xpcomutils and -sqlexplorer handler

>+ * The Initial Developer of the Original Code is
>+ * Shawn Wilsher <[e-mail]>.

You're not Shawn... :)


>+# The Original Code is 
>+#
>+# The Initial Developer of the Original Code is

Need these filled out.

Comment 8

[Shawn Wilsher :sdwilsh]

Comment on attachment 280426 [details] [diff] [review]
redone using xpcomutils and -sqlexplorer handler

Attack of the nits!

>Index: Makefile.in
>===================================================================
>--- Makefile.in	(revision 6698)
>+++ Makefile.in	(working copy)
>@@ -47,6 +47,7 @@
> INSTALL_EXTENSION_ID   = [e-mail]
> XPI_PKGNAME            = storage-explorer
> USE_EXTENSION_MANIFEST = 1
>+DIRS=components
nit: newline before, and format it just like ALL_LOCALES is done (spacing, slash, dirs on lines after it, etc).


>+const Cu = Components.utils;
>+const Ci = Components.interfaces;
>+const Cc = Components.classes;
>+
>+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
So, it'd be awesome if you could do comments like in this file:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/uriloader/exthandler/nsWebHandlerApp.js&rev=1.2#41

>+  classDescription : "storage-explorer",
nit: no space after property name

>Index: components/Makefile.in
and you actually don't even need this - see http://www.google.com/codesearch?hl=en&q=show:GLitOY8iItc:f9VdRXVnTe8:9H2urr5zFZk&sa=N&ct=rd&cs_p=http://trac.growl.info&cs_f=browser/branches/moz-extension/trunk/Makefile.in%3Fformat%3Draw&start=1

Comment 9

[u278084]

Attachment: Final version

(In reply to comment #6)
> entry is usually "m-" + type, so can we change the contractID type to
> "sqlexplorer" (to match the flag) and then entry : "m-sqlexplorer"?
I missed those. Thanks Mark.

(In reply to comment #7)
> You're not Shawn... :)
If you read it a certain way, it makes sense for Shawn's name to be there. But thinking it over, it will probably do good for my name to be in instead.

(In reply to comment #8)
> (From update of attachment 280426 [details] [diff] [review])
> Attack of the nits!
Hey, it's your code! You can ask for the patch in punch cards over fedex if you want to (but I'm not willing to go that far).

> nit: newline before, and format it just like ALL_LOCALES is done (spacing,
> slash, dirs on lines after it, etc).
I assume you meant the DIR variable. I took it out once I read the comment you made later on and I didn't need it anymore.

> So, it'd be awesome if you could do comments like in this file:
> http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/uriloader/exthandler/nsWebHandlerApp.js&rev=1.2#41
> 
ok.

> >+  classDescription : "storage-explorer",
> nit: no space after property name
ok.

> >Index: components/Makefile.in
> and you actually don't even need this - see
> http://www.google.com/codesearch?hl=en&q=show:GLitOY8iItc:f9VdRXVnTe8:9H2urr5zFZk&sa=N&ct=rd&cs_p=http://trac.growl.info&cs_f=browser/branches/moz-extension/trunk/Makefile.in%3Fformat%3Draw&start=1
Took out the Makefile entirely. Yay for less lines.

Comment 10

[Shawn Wilsher :sdwilsh]

Sorry for the delay!

Sending        Makefile.in
Adding         components
Adding         components/cmdline-handler.js
Transmitting file data ..
Committed revision 7370.