Closed Bug 56654 Opened 24 years ago Closed 16 years ago

command-line interface doesn't allow for arbitrary processing

Categories

(SeaMonkey :: General, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: shaver, Unassigned)

References

()

Details

(Keywords: arch)

Attachments

(2 files)

Currently, the command-line handler interface (nsICmdLineHandler and nsAppRunner) only permit command-line switches to set new default chrome URLs. That's not really enough for some cases, like UI language selection, -remote, etc. I've added a processArguments method to the nsICmdLineHandler interface, which will provide for arbitrary processing capability. I'll attach a patch, and test case, shortly. (If you're cc:d on this bug, you probably have a bug assigned to you that's related to command-line processing or arguments. Please read the patch and comment as to whether this makes your life easier, or harder, and whether additional interface changes would help.)
Status: NEW → ASSIGNED
Keywords: arch, patch
OS: Linux → All
Target Milestone: --- → mozilla0.9
I should add that this will let us clear all of the profile-manager-specifc crud out of the main application, and let it use this interface as well. Just to sweeten the pot a little. Bueller?
You all hate me.
Target Milestone: mozilla0.9 → mozilla0.9.1
Who's kidding whom?
Target Milestone: mozilla0.9.1 → mozilla0.9.2
17139: + // first, see if this handler wants to launch an app (it might just + // be setting some internal state missing ) in comment. This looks cool, but I don't think I'm confident enough to give an r= in this area.
Keywords: approval, review
Asa just told me that we shipped 0.9.2! Who knew?
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Target Milestone: mozilla0.9.3 → mozilla0.9.4
->0.9.5 since we have run out of time
Target Milestone: mozilla0.9.4 → mozilla0.9.5
rginda gets this, in order to guilt him a little bit.
Assignee: shaver → rginda
Status: ASSIGNED → NEW
Keywords: mozilla1.0
Target Milestone: mozilla0.9.5 → ---
This doesn't go deep enough, really. If we're going to expose a command-line registration interface to component developers in 1.0, and I really think we should, then we need to rip this baby up and start over. nsICmdLineHandler is a disaster, I think we all agree. I'll even take this bug back, to avoid nauseating rginda any more.
Assignee: rginda → shaver
Thank you Mike Shaver, thank you.
Since simple MAPI needs this bug to be fixed, changed milestone to m0.9.7.
Priority: P3 → P1
Target Milestone: --- → mozilla0.9.7
*** Bug 108766 has been marked as a duplicate of this bug. ***
Assigning to myself. The fix is required to land MAPI on trunk (meta bug 104672). Also the method "ProcessArguments" will be more generic as PRUint32 onCommandLineSwitch(in [array,size_is(len)] string argv, in PRUint32 len, in PRUint32 offset); For a detailed description, see bug 108766.
Assignee: shaver → kkhandrika
Blocks: 104672
Changed QA to Trix.
QA Contact: sairuh → trix
<kkhandrika@netscape.com> email bounces. Is this person still working on Mozilla? Should this bug be reassigned?
picking assignee from mapi bug
Assignee: kkhandrika → rdayal
Target Milestone: mozilla0.9.7 → mozilla0.9.8
rajiv wants to do the right thing, and not add any more hacks, but doesn't have the cycles to re-write all the existing cmd line handler stuff. how about this, for a place to start: rename nsICmdLineHandler as nsICmdLineHandlerObsolete. add the new interfaces as nsICmdLineHandler. Once we get the desired interface agreed on, and into the tree, we can add -mapi using it, and maybe rewrite -mail or -compose to use it, and add the necessary C++ code to use both nsICmdLineHandlerObsolete and nsICmdLineHandler (for now.) we'll mark nsICmdLineHandlerObsolete as depricated. then, we can log bugs on various owners to switch over from nsICmdLineHandlerObsolete to nsICmdLineHandler. once they're all done, we can remove nsICmdLineHandlerObsolete, and remove the extra C++ goop we've got for handling both. comments? if this sounds like a way to make progress, with out overloading rajiv, we should start working on the new interface.
I have to say, unfortunately, that I'm not surprised at _all_ that this got dropped on the floor, after we let MAPI land with a "one-off, temporary" hack. This was _exactly_ the problem I was concerned about when we discussed this in email, and I'm sad to see that my cynicism was warranted. Bah. As I said back in November: > I think that MAPI support isn't higher-priority than this architectural fix, > as far as Mozilla's concerned. I wouldn't mind 0.9.6 shipping without MAPI, > but I think that if we don't fix this now while the motivation is present it's > only going to get harder down the road. I don't want two interfaces. There's no need for them here, and we already have too many "nsIWhateverObsolete" concessions to use of unfrozen interfaces in the tree. Unfortunately, it seems that the only way to get people to do the right thing in our communal codebase is to make it really frigging hard to do the wrong thing, so maybe the way to move forward here is to just unilaterally break the interface so that the people managing Rajiv-or-whoever's Mozilla time see their way to allocating the cycles. (I tried logging bugs on various owners to get them to abandon NSGetFactory, which has been deprecated for something like two years now. 8 months later, I'm still waiting, so I'm not really willing to go that route again. And why would those owners have the cycles that Rajiv doesn't? This bug was opened because I had the cycles to improve the interface -- albeit incrementally, but that was back in _October_2000_ -- and you can read here how much luck I had even getting owners of affected code to respond to a WORKING PATCH.) I'm pretty disappointed that we ended up here, I have to say, though some of that disappointment is in myself, for failing to hold the line on this before. Happy new year.
Hi Mike, I understand your concerns. MAPI has landed but not with any temporary hack. The bug which has lead to this issue is still open. The current landing of MAPI would bring up the browser window which we want to avoid by fixing this bug. What do u suggest we should do besides re-writing the complete nsICmdLineHandler if u feel this idea of having another nsI..Obsolete is not good. We can try and use your patch at this moment and later decide to re-write nsICmdLineHandler as time permits. I was looking at the nsAppRunner code, there is a quite some code there that can be re-organized to be either part of or use nsICmdLineHandler functionality if we want to do it right. Plus there is the Profile Manager stuff u mentioned. And there could be more. Changing all that can have major impact and take quite some to get it through. However I believe enhancing the current implementation and not making it as nsI..Obsolete as Seth suggested will lead people to continue using the modified implementation. The 'Obsolete' word would lead people to atleast be cautious that it is going to change at some time !
> I have to say, unfortunately, that I'm not surprised at _all_ that this got > dropped on the floor I don't think anything has been dropped on the floor. actually, given the MAPI code has landed minus the objectionable MAPI code from the branch, and that mozilla has MAPI support, I'd say we're doing good. Now our problem is how to solve a MAPI bug that was fixed on the branch (by "any means") but now existings in mozilla (see #103785) > I don't want two interfaces Your explanation (and experience) on the pitfalls of nsIFooObsolete are noted. We'll go another route. > I'm pretty disappointed that we ended up here I'm not disappointed. The MAPI code that actually made it onto the trunk was reviewed and super-reviewed carefully. I'll talk with some managers, to see about getting some resources to fix it in a way that will allow it to be checked into the trunk. (meaning, fix the interface once and for all, and all the consumers.)
right now, I see two potential solutions: 1) spend the resources now, fix the interface and the consumers, which allows us to land and fix the bug on the trunk. 2) wait until netscape branches, netscape applies the hack to the branch, bug still exists on trunk. it doesn't make any sense for us to come up with a hack for the trunk, or a nsIFooObsolete, as it would be rejected (and rightly so) for background: the problem that we're trying to fix is "on blind send, we get a browser window" so if mozilla is registered as my MAPI appication, and it's not running when I do "send mail" from Word or another applications, I'll get a browser window.
<so maybe the way to move forward here is to just unilaterally break <the interface so that the people managing Rajiv-or-whoever's Mozilla time see <their way to allocating the cycles. You're welcome to try this, but my guess is that Seth's choice #2 may be the way to go. We'll have to see. I need more info on how long it would take to do this. Right now all of the data I have points to choice #2, which is fine.
Moving out. We'll fix what we need to on a branch and if we have time, we'll attack this problem later so we can land on the trunk.
Target Milestone: mozilla0.9.8 → mozilla1.2
Blocks: 104291
removed tiantian from cc (bad alias; sends mail to jhooker)
Blocks: 184790
Is this bug for allowing different components to register code or functions with the command line service so that they can parse the data themselves? If so, see bug 184789 about being able to register context-sensitive help with the command line service. Also, this will be made to block the command line META bug 184790.
Assignee: rdayal → nobody
QA Contact: trix
Target Milestone: mozilla1.2alpha → ---
This bug is seamonkey-only, and will be fixed when they switch over to suiterunner.
Assignee: nobody → general
Component: Cmd-line Features → General
Product: Core → Mozilla Application Suite
QA Contact: general
Depends on: suiterunner
Priority: P1 → --
Fixed by moving to toolkit.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: