Closed Bug 26761 Opened 25 years ago Closed 18 years ago

-help and -version not working on optimized win32 builds

Categories

(Core Graveyard :: Cmd-line Features, defect, P3)

x86
Windows 95
defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 355889
Future

People

(Reporter: alan-lists, Assigned: netdragon)

References

Details

(Keywords: helpwanted, platform-parity)

Attachments

(1 file, 10 obsolete files)

In the 2/5/00 build -help does not output anything. Could this have come up after the console text was removed?
*** Bug 26781 has been marked as a duplicate of this bug. ***
"-help" is still working (as well as it ever did). But the output goes to the console, which isn't there any more. So, to see the output, you'd have to use output redirection to see the output, e.g., "mozilla -help>logfile" then "type logfile". If we add support for "-console" (to force a console window) then I'll make "-help" imply "-console." See bug 26103.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → INVALID
I was wondering shouldn't this just be left open and depends on bug 26103? A feature of something like -help or /? (as in bug 23501) for command line support is helpful and in many products.
well, actually, -help is a little busted I'm rewriting how the command lines get handled (using XPCOM categories and the nsICmdLineHandler interface) I'm working on fixing the -help to be as good as it was before. (right now, you'll notice the help info is not complete.) law is right, on windows, we will have the -console problem, on windows (and mac, I guess.) to fix.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
I'll take this, since I need to fix -help for other reasons.
Assignee: don → sspitzer
Status: REOPENED → NEW
the console problem still remains. moving to m15.
Target Milestone: M15
now that -console is fixed, I can use it to get -help working again. accepting, will fix for m15.
Status: NEW → ASSIGNED
moving to m16.
Target Milestone: M15 → M16
This is just a reminder that besides -help, -h, -?..... The console output should also work for -v, -version also not sure there if there are other paramaters need that output to console also.
spam, changing selected qa contact on selected bugs from paulmac@netscape.com to claudius@netscape.com
QA Contact: paulmac → claudius
Seth, why do you own this? Could someone else be fixing it right now?
not going to happen for m16. making -h act like "-h -console" will not work for commercial builds. here's why: doing -console opens a dos console that goes away when the app quits. so doing -h -console on a commercial build will dump out help to the console, and then the console will immediately go away. law / mcafee, you interested in taking this one from me? moving to m18, this is not a m16 bug.
Target Milestone: M16 → M18
update from timeless in #mozilla, even "-h -console" doesn't do anything. I was hoping it would at least flash the console.
mozilla -h > outfile does nothing too. We need to support output to current stdin/stdout/stderr, redirection is nice and all, but if i'm debugging and open cmd.exe [nt4/w2k] I should be able to use it. [adding self to cc]
over to me...if claudius doesn't mind ;).
Component: XP Apps → XP Apps: Cmd-line Features
QA Contact: claudius → sairuh
Anyone on the CC list care to own this? Seth won't be fixing this in beta3. If no volunteers, then I'll be moving this to FUTURE.
"mozilla -h > output" *does* work, assuming Mozilla is not already running (if it is, then too bad, you can't need too much help in that case :-). The problem with -console remains. There is not much we can do, since Windows is Windows and it gives us no access to the console of the command.com/cmd.exe that launched us. The only option is something like: Insert a "press any key" sort of thing after dumping the help info (windows only, only if MOZ_WINCONSOLE==0). That way, the app doesn't exit and the console doesn't go away. I'll also make "-help" imply "-console" (currently, "-help" blocks the "-console" code from getting called so I'll have to rearrange this a little). If that's acceptable, I'll take this for beta3. If we insist on some more utopian solution, then definitely FUTURE it.
I like the option of "Press any key". law, if you do this, it might be good to add code at the same time to count the number of lines to be displayed in the console with -help. So if it is greater then 24 or 25 then you would get a "press any key" after every 25 lines on the screen then another at the very end so we can see the last few. Thiw ould let us see all command line paramaters if we get more then a screen full.
I don't have time (now) to take this, but many of the comments are not entirely correct. Case one: regedit.exe where command.com (win9x not NT) is configured (via pif) to lie ~windows is not running~ result: help in console. The solution is probably to change the loader code (possibly the REAL main) to do some parsing before it starts using winmain. Another application that might be similar is "start" (actually i'm not quite sure it's an application) F:\backup\Win98\WINDOWS\COMMAND\START.EXE <-- win9x No such critter in w2k. F:\backup\Win98\WINDOWS\COMMAND>ver Microsoft Windows 2000 [Version 5.00.2195] F:\backup\Win98\WINDOWS\COMMAND>.\START.EXE /? Runs a Windows program or an MS-DOS program. START [options] program [arg...] START [options] document.ext /m[inimized] Run the new program minimized (in the background). /max[imized] Run the new program maximized (in the foreground). /r[estored] Run the new program restored (in the foreground). [default] /w[ait] Does not return until the other program exits. I think that help is intuitive enough. wrt mozilla -h > file. Again I nearly always use non debug builds and in my experience it does not work. Debug builds are special but...our release product isn't exactly supposed to be a debug build. Press any key to continue is a reasonable short term fix. So is the win3.1/win9x pif setting "don't close window on exit" Mozilla could change the window titled to Finished and just wait for someone to close it.
Bleh. I missed a comment. windows nt allows for the console to have varying sizes so does 9x. iirc on 9x it can be 24,25 or 50 lines. When we use -console we seem to create a scrolling window (well I'm in 2000 so maybe this is the os's action). If we actually create the window and the scrollbars then there's no need for the multiple press any keys. If we don't then use some functions to find out how many lines the window shows (see cmd.exe's [conflicting] implementations: D:\Program Files\Netscape\Mozilla\new\bin>help For more information on a specific command, type HELP command-name. ASSOC Displays or modifies file extension associations AT Schedules commands and programs to run on a computer. ATTRIB Displays or changes file attributes. BREAK Sets or clears extended CTRL+C checking. CACLS Displays or modifies access control lists (ACLs) of files. CALL Calls one batch program from another. CD Displays the name of or changes the current directory. CHCP Displays or sets the active code page number. CHDIR Displays the name of or changes the current directory. CHKDSK Checks a disk and displays a status report. CHKNTFS Displays or modifies the checking of disk at boot time. CLS Clears the screen. CMD Starts a new instance of the Windows 2000 command interpreter. COLOR Sets the default console foreground and background colors. COMP Compares the contents of two files or sets of files. COMPACT Displays or alters the compression of files on NTFS partitions. CONVERT Converts FAT volumes to NTFS. You cannot convert the current drive. COPY Copies one or more files to another location. DATE Displays or sets the date. DEL Deletes one or more files. DIR Displays a list of files and subdirectories in a directory. DISKCOMP Compares the contents of two floppy disks. DISKCOPY Copies the contents of one floppy disk to another. DOSKEY Edits command lines, recalls Windows 2000 commands, and creates macros. ECHO Displays messages, or turns command echoing on or off. ENDLOCAL Ends localization of environment changes in a batch file. ERASE Deletes one or more files. EXIT Quits the CMD.EXE program (command interpreter). FC Compares two files or sets of files, and displays the differences between them. FIND Searches for a text string in a file or files. FINDSTR Searches for strings in files. FOR Runs a specified command for each file in a set of files. FORMAT Formats a disk for use with Windows 2000. FTYPE Displays or modifies file types used in file extension associations. GOTO Directs the Windows 2000 command interpreter to a labeled line in a batch program. GRAFTABL Enables Windows 2000 to display an extended character set in graphics mode. HELP Provides Help information for Windows 2000 commands. IF Performs conditional processing in batch programs. LABEL Creates, changes, or deletes the volume label of a disk. MD Creates a directory. MKDIR Creates a directory. MODE Configures a system device. MORE Displays output one screen at a time. MOVE Moves one or more files from one directory to another directory. PATH Displays or sets a search path for executable files. PAUSE Suspends processing of a batch file and displays a message. POPD Restores the previous value of the current directory saved by PUSHD. PRINT Prints a text file. PROMPT Changes the Windows 2000 command prompt. PUSHD Saves the current directory then changes it. RD Removes a directory. RECOVER Recovers readable information from a bad or defective disk. REM Records comments (remarks) in batch files or CONFIG.SYS. REN Renames a file or files. RENAME Renames a file or files. REPLACE Replaces files. RMDIR Removes a directory. SET Displays, sets, or removes Windows 2000 environment variables. SETLOCAL Begins localization of environment changes in a batch file. SHIFT Shifts the position of replaceable parameters in batch files. SORT Sorts input. START Starts a separate window to run a specified program or command. SUBST Associates a path with a drive letter. TIME Displays or sets the system time. TITLE Sets the window title for a CMD.EXE session. TREE Graphically displays the directory structure of a drive or path. TYPE Displays the contents of a text file. VER Displays the Windows 2000 version. VERIFY Tells Windows 2000 whether to verify that your files are written correctly to a disk. VOL Displays a disk volume label and serial number. XCOPY Copies files and directory trees. D:\Program Files\Netscape\Mozilla\new\bin>help start Starts a separate window to run a specified program or command. START ["title"] [/Dpath] [/I] [/MIN] [/MAX] [/SEPARATE | /SHARED] [/LOW | /NORMAL | /HIGH | /REALTIME | /ABOVENORMAL | /BELOWNORMAL] [/WAIT] [/B] [command/program] [parameters] "title" Title to display in window title bar. path Starting directory B Start application without creating a new window. The application has ^C handling ignored. Unless the application enables ^C processing, ^Break is the only way to interrupt the application I The new environment will be the original environment passed to the cmd.exe and not the current environment. MIN Start window minimized MAX Start window maximized SEPARATE Start 16-bit Windows program in separate memory space SHARED Start 16-bit Windows program in shared memory space LOW Start application in the IDLE priority class NORMAL Start application in the NORMAL priority class HIGH Start application in the HIGH priority class REALTIME Start application in the REALTIME priority class Press any key to continue . . . ... I prefer help's approach (this window's buffer is 300 lines tall and that's what I care about, not the visible 54 lines; I want apps to check *the buffer size*) [note: help start is help on w2k's cmd.exe's native start not the one used in the previous example]
Law wrote: The problem with -console remains. There is not much we can do, since Windows is Windows and it gives us no access to the console of the command.com/cmd.exe that launched us. I just remembered: viewer.exe uses the cmd.exe that launched it for debug output. Maybe I should go in and compare viewer's source to apprunner (mozilla)'s. http://lxr.mozilla.org/seamonkey/source/webshell/tests/viewer/nsWinMain.cpp /*148*/ int main(int argc, char **argv) { //... app->Initialize(argc, argv); //... } http://lxr.mozilla.org/seamonkey/source/webshell/tests/viewer/nsViewerApp.cpp /*306*/ nsViewerApp::Initialize(int argc, char** argv) { //... rv = ProcessArguments(argc, argv); //... } /*519*/ nsViewerApp::ProcessArguments(int argc, char** argv) { //... PrintHelpInfo(argv); //... } /*441*/PrintHelpInfo(char **argv) { fprintf(stderr, "Usage: %s [options] [starting url]\n", argv[0]); //... }
i run into this every time i run an optimized build on win32, so am adding mostfreq so that it'll at least get on my radar quickly when i run a query.
Keywords: mostfreq, pp
Removing mostfreq - this bug does not qualify for this keyword under the established criteria. Gerv
Keywords: mostfreq
adding se-radar to status so that i can find this bug more easily.
Whiteboard: se-radar
Summary: -help not working on win32 2/5/00 build → -help not working on win32
Netscape Nav triage team: this is not a Netscape beta stopper.
Keywords: nsbeta1-
Whiteboard: se-radar
from bug 14003: this bug is preventing -version from working on win32 optimized bits. note to self: once this is fixed, doublecheck -version.
this should probably go to alecf, law or mcafee.
Reassigning to self. All I'll probably do is get -help to spawn a console and maybe add a Sleep() call so the user has a chance to read it.
Assignee: sspitzer → law
Status: ASSIGNED → NEW
Target Milestone: M18 → ---
it would be better if -help or -version didn't bring up a console.
Summary: -help not working on win32 → -help and -version not working on optimized win32 builds
If the executable file wasn't started from a command line (i.e. a shortcut) then it probably won't have a console to display in, unless -help implies -console. Other than that any stdio should go to the console that the app started in its too confusing otherwise. Is this still helpwanted?
http://msdn.microsoft.com/library/periodic/period96/S569.htm looks interesting, as do some other links mainCRTStartup and WinMainCRTStartup might be enough to solve our problems. I'll work on a test application soon. My current test concept is a main() which logs an event in the application event log.
*** Bug 70447 has been marked as a duplicate of this bug. ***
After much thinking, I believe the options would be one of the following: 1) To create a simple console application to hold the console window, and link it into the application as binary data. Then you can load the executable straight from mozilla, write it to a file, run it, close mozilla. 2) Include another file in the dir named console.bin and run that. 3) If its possible, load a DLL that is independent of mozilla that opens a console, and registers a control handler with SetConsoleCtlHandler, and then close mozilla. 4) Make mozilla always a console application, and have it close the console if its not needed. Console will flicker? Console applications can open regular windows, I tested that. Don't know if you can close the console in a console application without closing the application. If you FreeConsole before the application closes, the console stays open even if the application is no longer open. I'm gonna play around with this a little more. It was timeless's idea - and will be the way to do it if you can somehow close the console if mozilla goes into gui mode. 5) Open the console and have Mozilla stay open until the console is closed. If Mozilla was opened again, it would have to know that the currently open console isn't really the gui mode of mozilla.
Sorry, unfortunately, I was wrong on #4 - it was only when I ran it within MSVC.
Notice: #4 does let you write to the command prompt though when run from cmd.exe.
1-5 don't make any sense. in English, we can easily make mozilla a console application. w32 allows us to release the console if we like. The only big issue is that when we want to hide the console it flickers. I think we can find out when our console is fresh. if it's fresh we can hide it temporarily [first step, reduces flash] -- probably minimizing it. result: we decide whether we need to display output to the console, if we do, we show the console. either way we put output to the console. if we don't need to use the console, we release it.
One of the solutions, was make a console application that does nothing but shows the parameters or help for each parameter. Then you can pack it into the .exe and load it as a resource, turn it into memory, save it to a temp file and run it. This was successful and fast for me on my test. Screenshots to be attached, along with a directory containing two .dsw files for msvc++. One .dsw creates the commandline.exe file. The other makes commandlinegui.exe. commandlinegui.exe contains commandline.exe as a resource and loads it from its .exe file using windows API. It then saves it to a file and runs that file. The file is placed in the temp dir. Try it, it works!
Attached image "commandlinegui.exe /?" (obsolete) (deleted) —
Attached image "commandlinegui.exe" (obsolete) (deleted) —
Note that these are .dsw files to be loaded by MSVC++ 6. First load commadline.dsw and build. Then load commandlinegui.dsw and build. they go to the debug dir and try: commandlinegui commandlinegui /? Notice the difference in what happens and also notice that the window stays after the original program ends.
In conclusion, this would allow it to display the parameters without it flickering when starting up in normal mode. The only drawback is the parameters will be loaded in another window from the command prompt. There is no way around that since the gui app (Mozilla) isn't attached to the console (command prompt) on startup because of the way Windows works. This is ok, anyway since it can be kept as a reference while entering commands. Also calling Mozilla with paramaters from run in the start menu will load a console window, which is nice.
Would it be sufficient to make mozilla.exe -help open about:commandline (bug 60085) in a new Navigator window?
Jesse: I think that would be annoyingly slow. What I just showed will work and I believe is the best way. That way you get no flickering. A console application is very small and won't change the size of mozilla very much. Timeless: I'll explain it better: Definitions: gui mode - regular browser mode console / console mode / console app - just display command line params in a window 1) Link the console app into Mozilla.exe as a resource and write to a temp file after loading Mozilla and run the temp file - this is what I just showed how to do. 2) Simply, just include a file in the mozilla dir called console.bin or something which is an executable file that has been renamed to a non-executable extension. Mozilla can then do WinExec("console.bin",...). 3) Use rundll to load a dll that loads the console so the dll doesn't close when mozilla closes (I think). 4) Make mozilla always a console application, and have it close the console if its not needed. Console will flicker. Console applications can open gui windows, I tested that. You still need to close the app before you run it again. Hopefully the console window stays open. 5) Open the console and have Mozilla (in console mode) stay open until the console is closed. If mozilla.exe is opened again without closing the previous instance, it would have to know that the currently open console isn't the gui mode of mozilla and therefore won't interfere with the gui mode. Also, the command line params don't necessarily have to be shown in a console window. They could be showed in a regular window that allows copying to the clipboard, although I think that would look sloppy.
BTW: 1-5 are all different options and not steps.
Attached file c++ file for commandline.exe (obsolete) (deleted) —
There's localization problems with this Rube Goldberg scheme (no offense).
afaik this whole process is still totally unnecessary.
If you could somehow make a console DLL that is loaded by Mozilla but stays active (with a timer) after Mozilla exits, that would work too.
That way the .dll could be just stuck into the .dll dir and wouldn't have to be explicitly linked into the code.
Marking nsbeta1- bugs as future to get off the radar.
Target Milestone: --- → Future
*** Bug 90575 has been marked as a duplicate of this bug. ***
We need to have some way for Windows users to see what commandline options are available.
Keywords: nsbeta1-mozilla1.0, nsbeta1
Mike Young: see also bug 60085, [RFE] about:commandline.
Shouldn't this be fixed by 1.0?
Blocks: 104166
Still, neither of these are any better than timeless's wrapping WinMain in main in a console app idea since there still is a flicker if trying to simply do gui.
Bug 60085 is about adding about:commandline. Due to the limitations of Windows, I feel we should have -help bring up about:commandline and -version bring up about: and set this bug dependant on both those bugs. AFAIK, there is no good way around this dilemma other than that. The only four choices are: 1) Bring up another console or window. 2) Make it a console application. It will flicker before you hide the console if not using the help info. 3) Do the mozilla.exe, mozilla.com thing msdev does (info in earlier comment today). If someone types "mozilla.exe -help" then it won't work, they would have to type "mozilla -help" 4) Somehow create a hybrid subsystem that is neither console nor gui. We would have to somehow feed this to linker. I have no clue how we would do this. I had limited success with replacing WinMainCRTStartup by linker option ENTRY:MyStartup. The problem was, if I created it as a console app, Windows still made the console. If I made it as a gui app, I still didn't have access to the console even if I called mainCRTStartup directly. IMHO, let's stop beating ourselves up over Windows shortcomings and just fix the about:commandline and about: for -help and -version
Microsoft must have fixed this in Windows XP Surprisingly, mozilla -help works in Windows XP!
I submitted a patch for about:commandline. For Windows systems except Windows XP, we can open up a browser window to about:commandline. Patch in Bug 60085 for about:commandline.
Running build 2002042608 on Windows XP Professional, but running 'mozilla - help' or 'mozilla - version' from the command prompt still doesn't work for me.
That's strange. I wonder why. My friend noticed the same thing as you. I'm thinking it has something to do with the fact that I have msvc++ service pack 5. If not that, it might be some windows update I have that others don't. Can anyone with msvc++ and sp5 check this?
I just realized the reason is cause I was doing it in my tree, on my debug build. Hmmmm. I think someone has mentioned that before. I forget why it works though. Sorry for the unintentional spam.
Actually, I have an idea that I am working on. I'll post a patch tomorrow. Basically I've changed the DumpHelp function to call some APIs like DisplayHelpText and DisplayHelpOption rather than use printfs. Then I've created two new functions called BeginHelp and EndHelp. The idea is that if you are going to use the DisplayHelp APIs, you do something like this: BeginHelp DisplayHelp... DisplayHelp... . . . EndHelp This allows a platform to do setup in BeginHelp and then process the help statements all at once in EndHelp. For instance on Windows right now, I'm doing an allocation in BeginHelp and then in EndHelp I use a MessageBox to display the help and then free the buffer. This solution gives a great deal of platform flexibility, while leaving unix platforms working exactly the way they used to. We could even create a Windows console in BeginHelp, and then display the help in that console.
Creating a console would look ugly if you start Mozilla from a console, because there is (to my knowledge) no way to use an existing console. Also, you'd have to ensure that the console doesn't go away directly after displaying the message, so that the user can read the help :)
Attached patch First attempt at a patch (obsolete) (deleted) — Splinter Review
OK, this implements my idea. Now when you do mozilla -version or mozilla -help, you get a message box on Windows. Unix and others work the same way the used to. The only problem I am running into is alignment within the message box. -jsconsole and -installer both have 10 letters, but completely different sizes with the default windows font. You'll see it when you run with the patch. I think this is a pretty good idea to do this. Looking for opinions.
*** Bug 159726 has been marked as a duplicate of this bug. ***
mkaply: I tried your patch, and it is a good solution until we are able to modify the Mozilla stub. The only thing I thought about when I tested it is that you can't select or copy the text. Is there any way we could make the text selectable? If dialogs don't allow it, we could make this a window instead. We could also possibly put a button in that says "Copy to clipboard". People might want to copy it all to put on a web page, text file or something else. I'm going to modify this patch so that this is possible.
mkaply: nsApprunner.cpp is a cross-platform file. You need to put #if statements around code you don't want compiling in Linux (such as the MessageBox). I tried compiling on Linux and had the following errors: ../../../xpfe/bootstrap/nsAppRunner.cpp:518: warning: ISO C++ forbids declaration of `BeginHelp' with no type ../../../xpfe/bootstrap/nsAppRunner.cpp: In function `int BeginHelp ()': ../../../xpfe/bootstrap/nsAppRunner.cpp:521: warning: no return statement in function returning non-void ../../../xpfe/bootstrap/nsAppRunner.cpp: At top level: ../../../xpfe/bootstrap/nsAppRunner.cpp:524: warning: ISO C++ forbids declaration of `EndHelp' with no type ../../../xpfe/bootstrap/nsAppRunner.cpp: In function `int EndHelp (char *)': ../../../xpfe/bootstrap/nsAppRunner.cpp:525: `HWND_DESKTOP' undeclared (first use this function) ../../../xpfe/bootstrap/nsAppRunner.cpp:525: (Each undeclared identifier is reported only once for each function it appears in.) ../../../xpfe/bootstrap/nsAppRunner.cpp:525: `MB_OK' undeclared (first use this function) ../../../xpfe/bootstrap/nsAppRunner.cpp:525: `MessageBox' undeclared (first use this function) ../../../xpfe/bootstrap/nsAppRunner.cpp:527: warning: no return statement in function returning non-void ../../../xpfe/bootstrap/nsAppRunner.cpp: At top level: ../../../xpfe/bootstrap/nsAppRunner.cpp:530: warning: ISO C++ forbids declaration of `DisplayHelpTextWithString' with no type ../../../xpfe/bootstrap/nsAppRunner.cpp: In function `int DisplayHelpTextWithString (const char *, const char *)': ../../../xpfe/bootstrap/nsAppRunner.cpp:542: warning: no return statement in function returning non-void ../../../xpfe/bootstrap/nsAppRunner.cpp: At top level: ../../../xpfe/bootstrap/nsAppRunner.cpp:545: warning: ISO C++ forbids declaration of `DisplayHelpText' with no type ../../../xpfe/bootstrap/nsAppRunner.cpp: In function `int DisplayHelpText (const char *)': ../../../xpfe/bootstrap/nsAppRunner.cpp:552: warning: no return statement in function returning non-void ../../../xpfe/bootstrap/nsAppRunner.cpp: At top level: ../../../xpfe/bootstrap/nsAppRunner.cpp:555: warning: ISO C++ forbids declaration of `DisplayHelpOption' with no type ../../../xpfe/bootstrap/nsAppRunner.cpp: In function `int DisplayHelpOption (const char *, const char *)': ../../../xpfe/bootstrap/nsAppRunner.cpp:579: warning: no return statement in function returning non-void make: *** [nsAppRunner.o] Error 1
We could certainly do that and I was going to have to do it for OS/2. Create a dialog with a text area in it that displays the help information.
- This patch doesn't break Linux. Not only that, it speeds up the help text display thanks to mkaply's idea of printing it all out at the end which I also put for the Linux code. - This patch also shows the text aligned correctly - It allows for copyable text on Win32 -help in a nice-looking Window mkaply: So OS/2 doesn't show the info either? Can you make this work in OS/2, also? (I don't have OS/2)
Attachment #26491 - Attachment is obsolete: true
Attachment #26492 - Attachment is obsolete: true
Attachment #26493 - Attachment is obsolete: true
Attachment #26501 - Attachment is obsolete: true
Attachment #26502 - Attachment is obsolete: true
Attachment #93299 - Attachment is obsolete: true
Attachment #108851 - Attachment description: diff from mozillaroot → latest diff, everything but OS/2, works for Linux
Attachment #108851 - Flags: review?(cbiesinger)
See also bug 184594 about timeless's idea about making a new application stub to replace the loading code (before main or Winmain even get called) which is something we can try do in the future but is not a priority since michael kaply came up with a sufficient work-around for now. Let's get his idea into the tree! :-)
I also have updated the patches on Bug 60085 (about:commandline).
Comment on attachment 108851 [details] [diff] [review] latest diff, everything but OS/2, works for Linux (fwiw, writing an entirely new stub seems _so_ wrong to me. but ok, whatever, I do not know what that would involve... I am guessing a lot, and I think we don't want this) + helpbuffer = (char*)malloc(65536); any reason for exactly 64 KB? +#endif //XP_Win wrong case. (should be XP_WIN). Why bother allocating helpbuffer on all platforms if you're only using it on XP_WIN? (const char *)commandLineArg do me a favor and use .get() instead of the typecast. oh yeah and above in the function, maybe test emptyness with .IsEmpty() rather than typecast to const char* and checking non-null-ness. - // this works, but only after the components have registered. so if you drop in a new command line handler, -help - // won't not until the second run. - // out of the bug, because we ship a component.reg file, it works correctly. So. Any reason for removing that comment? That said, get module-owner review for this as well (also law I believe)
Attachment #108851 - Flags: review?(cbiesinger) → review?
Attached patch Final fix? (obsolete) (deleted) — Splinter Review
This patch also fixes bug 60265 biesi: The reason I allocated helpbuffer was because I rerouted even Linux info to it. It has since been renamed though. - Changed to use nsCString - Replaced the DisplayHelpTextWithString and DisplayHelpText with var arg AddCmdHelpText that uses PR_vsnprintf - Moved the -help info over a bit so its not more than 80 chars wide (but version is at least with a developer build) - Moved the functions to be closer together - Removed malloc reference - Fixed XP_Win What does this mean? - // this works, but only after the components have registered. so if you drop in a new command line handler, -help // won't not until the second run. out of the bug, because we ship a component.reg file, // it works correctly. Why don't we put this into the tree, and then have the OS/2 stuff in a separate bug?
Attachment #108851 - Attachment is obsolete: true
Comment on attachment 108991 [details] [diff] [review] Final fix? er. So there is exactly _one_ case which requires printf? + // NOTE: The above comment is poorly written. how so, looks perfectly fine to me. + #ifdef MOZ_WIDGET_GTK I think some compilers (preprocessors) have problems with ifdefs that don't start at the beginning of the line, please remove the space.
I realize I wrote the original patch, but my indent was way off. We should use two space indentation. Also I have the OS/2 piece of this code done as well.
Attached patch OS/2 stuff and indentation fixed (obsolete) (deleted) — Splinter Review
That comment IS poorly written. Unless you know what this means: won't not until the second run.
Attachment #108991 - Attachment is obsolete: true
// this works, but only after the components have registered. so if you drop in a new command line handler, -help // won't not until the second run. // out of the bug, because we ship a component.reg file, it works correctly. This is my take on the comment.... ("won't not" == won't know, "out of the bug" == out of the box) Command-line help displays the proper help information, but only after components have been registered. If a component with a command-line handler is added its help information will not display application has been run once without the "-help" parameter, because the help text is displayed before component registration occurs. Out of the box all help information is displayed correctly due to the included component.reg file.
the right thing to do is to register components <g>, this is actually rather easy to do.
mkaply: Is the text also selectable in OS/2?
yes. We use a text area the same as Windows.
Great! Ok, lemme modify the comment that doesn't make sense, and then let's get this baby in!
Correction to my interpretation... 'If a component with a command-line handler is added its help information will not display application has been run once without the "-help" parameter' I forgot "until the" between "display" and "application".
Attached patch diff -u from mozilla/.. (obsolete) (deleted) — Splinter Review
timeless: You said you would registration issue in #mozilla (and thanks to Dean Tessman I finally understand it. I filed a separate bug 184979 just for you. I modified the comment. Tested it on Win32 and Linux, no problems. Can I go to sleep now? ;-)
Comment on attachment 109053 [details] [diff] [review] OS/2 stuff and indentation fixed Forgot to mark previous obsolete
Attachment #109053 - Attachment is obsolete: true
Comment on attachment 109096 [details] [diff] [review] diff -u from mozilla/.. Asking law for review
Attachment #109096 - Flags: review?(law)
Comment on attachment 109096 [details] [diff] [review] diff -u from mozilla/.. My opinion is that this is way too much code for little benefit. I don't think it is worth it. That said, I don't really "own" this code anymore so I will not object if somebody goes ahead and checks it in. Also, I'd like to find somebody else to take over ownership of this code once and for all since I can't give it the attention it deserves.
Attachment #109096 - Flags: review?(law) → review-
Note that we have only added about 700 lines of code. and about 10k to the source file.
Comment on attachment 109096 [details] [diff] [review] diff -u from mozilla/.. Requesting review from jag. Notice that even though this is a large diff, at least half of it is actually removing code.
Attachment #109096 - Flags: review- → review?(jaggernaut)
Mkaply: This patch has shortened the nsAppRunner.cpp code to only an added 200 lines or so by making it a dialog stored in splash.rc (resource script) and using the DialogBox API function instead of being created through the CreateWindow API function. The actual patch file isn't any different in size (because it spans more files), but there is less code added. Please do the same (in splashos2.rc and nsAppRunner.cpp) so that this adds even less code.
Attachment #109096 - Attachment is obsolete: true
Comment on attachment 110291 [details] [diff] [review] New patch - Mkaply, please update >+static nsCString gCmdHelpBuffer; must it really be static? :( >+#if defined(XP_WIN) || defined(XP_OS2) >+ #define NS_CMDHELP_NEWLINE "\r\n" >+#else //XP_WIN >+ #define NS_CMDHELP_NEWLINE "\n" >+#endif //XP_WIN what about macos classic? (yes, i know no one in their right mind would do care, so?) also, either remove the //XP_WIN or fix it to be accurate (i'd remove it) >+ switch (uMsg) >+ { ... >+ case WM_INITDIALOG: scope variable definitions: { // <-- like this >+ // Set up the window's icons >+ HICON hMozillaIconLg = (HICON) LoadIcon( GetModuleHandle( NULL ), IDI_APPLICATION); ... >+ return TRUE; how exactly do you expect code to reach this break? >+ break; (remove it) >+ } >+static void AddCmdHelpText(const char * format, ...) >+{ >+ // It would be nice if nsPrintfCString has a constructor that >+ // accepted a va_list as its second param. valists suck. don't use them. they're bad bad bad. two alternatives which are better and safer: 1. an xpcom array which indicates length, especially when your callers will only pass in strings. 2. an overloaded function since at compile time you have to know the number of possible parameters -- you can't dynamically construct a valist :o. >+ nsCOMPtr<nsISimpleEnumerator> e; >+ rv = catman->EnumerateCategory(COMMAND_LINE_ARGUMENT_HANDLERS, getter_AddRefs(e)); >+ if(NS_SUCCEEDED(rv) && e) { don't deep nest. use |if (!e) return;| instead. also please use whitespace consistently |if(..| v. |if (..| >+ while (PR_TRUE) { i'm not a fan have while (1), usually there's a better condition, in this case it's |NS_SUCCEEDED(e->hasMoreElements(&more)) && more| in fact, what you're doing violates the nsISimpleEnumerator contract: Although hasMoreElements() can be called independently of getNext(), getNext() must be pre-ceeded by a call to hasMoreElements(). copied from: http://unstable.elemental.com/mozilla/build/latest/mozilla/xpcom/dox/interfacen sISimpleEnumerator.html#a1 but also available in the real idl file. >+ nsCOMPtr<nsISupportsCString> catEntry; >+ rv = e->GetNext(getter_AddRefs(catEntry)); >+ if (NS_FAILED(rv) || !catEntry) break; don't bother with the rv. catEntry will only be true if GetNext succeeds. >+ nsCAutoString entryString; >+ rv = catEntry->GetData(entryString); >+ if (NS_FAILED(rv) || entryString.IsEmpty()) break; same idea >+ nsXPIDLCString contractidString; >+ rv = catman->GetCategoryEntry(COMMAND_LINE_ARGUMENT_HANDLERS, >+ entryString.get(), >+getter_Copies(contractidString)); >+ if (NS_FAILED(rv) || !((const char *)contractidString)) break; again, don't bother with the rv. don't use (const char *) either, use IsEmpty() >+ nsCOMPtr <nsICmdLineHandler> handler(do_GetService((const char *)contractidString, &rv)); don't use (const char*), use .get() you don't actually use rv, so don't ask for it. >+ if (handler) { why not |if (!handler) continue;| ? >+ nsXPIDLCString commandLineArg; >+ rv = handler->GetCommandLineArgument(getter_Copies(commandLineArg)); >+ if (NS_FAILED(rv)) continue; >+ >+ nsXPIDLCString helpText; >+ rv = handler->GetHelpText(getter_Copies(helpText)); don't bother with rv. >+ if (NS_FAILED(rv)) continue; do |if (commandLineArg.IsEmpty()) continue;| >+ if (!commandLineArg.IsEmpty()) { >+ PRBool handlesArgs = PR_FALSE; >+ rv = handler->GetHandlesArgs(&handlesArgs); don't bother with rv. >+ if (NS_SUCCEEDED(rv) && handlesArgs) { >+ commandLineArg += NS_LITERAL_CSTRING(" <url>"); >+ } >+ >+ AddCmdHelpOption(commandLineArg.get(), (const char *)helpText); use .get() >+ AddCmdHelpOption("-nosplash", "Disable splash screen."); >+#if defined(XP_WIN) || defined(XP_OS2) >+ AddCmdHelpOption("-quiet", "Disable splash screen."); >+#endif /* XP_WIN || XP_OS2 */ >+#endif /* MOZ_ENABLE_REMOTE */ this nesting sucks, please unnest it (i don't care whose fault it is)
mkaply: Can you fix the issues that timeless spoke of that need fixing when you do the OS/2 portion please? > (From update of attachment 110291 [details] [diff] [review]) > >+static nsCString gCmdHelpBuffer; > must it really be static? :( Its used by global functions. The only other option is to pass it around in params. > >+#if defined(XP_WIN) || defined(XP_OS2) > >+ #define NS_CMDHELP_NEWLINE "\r\n" > >+#else //XP_WIN > >+ #define NS_CMDHELP_NEWLINE "\n" > >+#endif //XP_WIN > what about macos classic? (yes, i know no one in their right mind would do > care, so?) We can file another bug on that. Maybe mkaply can mention that in a comment within that part of the code. > also, either remove the //XP_WIN or fix it to be accurate (i'd remove it) Agreed. > >+ return TRUE; > how exactly do you expect code to reach this break? > >+ break; > (remove it) > + } If the break isn't there, then removal of the return TRUE might have unexpected results and cause a regression if its not caught if someone adds more statements. In this case, I think the questionable redundancy is with merit. >+static void AddCmdHelpText(const char * format, ...) >+{ >+ // It would be nice if nsPrintfCString has a constructor that >+ // accepted a va_list as its second param. > valists suck. don't use them. they're bad bad bad. Do you have a reason other than that they are bad? They are also used in some of the PR string classes. > >+ nsCOMPtr<nsISimpleEnumerator> e; > >+ rv = catman->EnumerateCategory(COMMAND_LINE_ARGUMENT_HANDLERS, > getter_AddRefs(e)); > >+ if(NS_SUCCEEDED(rv) && e) { > don't deep nest. use |if (!e) return;| instead. > also please use whitespace consistently |if(..| v. |if (..| > + while (PR_TRUE) { > i'm not a fan have while (1), usually there's a better condition, in this case > it's |NS_SUCCEEDED(e->hasMoreElements(&more)) && more| > > in fact, what you're doing violates the nsISimpleEnumerator contract: > Although hasMoreElements() can be called independently of getNext(), getNext() > must be pre-ceeded by a call to hasMoreElements(). That function was already there. Oh well.. I guess we can fix it. Sigh. > >+ AddCmdHelpOption("-nosplash", "Disable splash screen."); > >+#if defined(XP_WIN) || defined(XP_OS2) > >+ AddCmdHelpOption("-quiet", "Disable splash screen."); > >+#endif /* XP_WIN || XP_OS2 */ > >+#endif /* MOZ_ENABLE_REMOTE */ > >this nesting sucks, please unnest it (i don't care whose fault it is) Agreed. And if you look at the code before and after it, it sucks even more.
Attachment #108851 - Flags: review?
Attachment #109096 - Flags: review?(jaggernaut)
*** Bug 199827 has been marked as a duplicate of this bug. ***
*** Bug 207252 has been marked as a duplicate of this bug. ***
Nothing has been done even though this bug was filed in the year 2000? This is a really problematic thing. Not being able to query the commandline options _at the commandline_ for a program that takes commandline options? The installer -h works just fine btw...
Incorrect. If your read this bug, things have been done. I have thought about making a new loader for Mozilla to replace the gui loader that microsoft provides, but for the meantime, we have the patch that I made. http://bugzilla.mozilla.org/attachment.cgi?id=110291&action=view That has been rotting. So don't say nothing hasn't been done. That's insulting.
My apologies that is not what I meant to imply, English is not my first language. What I meant was that even though there had been extensive discussion and a patch had been supplied, long time ago, nothing of this had made it into Mozilla 1.4 which is what I am using at the moment.
I agree with Veronica. Brian and other Mozilla developers, stop getting all touchy about people's comments. Veronica stated quite clearly that this issue is over 3 years old and regardless of the fact you submitted a patch the fact remains the issue is *still* open and unresolved. Mozilla developers: someone has to take charge of this issue and carry it through to the end. Bill Law has ownership of this issue. Is he still active? Why isn't this issue getting resolved? All too often you guys shoot the messenger, and I think it's about time you stop it.
law is gone, for a year now I think.
Law is gone, so I'll take this... I'll have to make sure the patch still works and such. I won't do any OS/2 stuff. We can handle that seperately. Piggybacking the OS/2 stuff on the same patch doesn't seem like the most efficient way to do it. Eventually, I'm going to write a new bootloader to replace the generic Win32 bootloader, and this issue should magically go away (at least for Win32), but this is something that can be done more immediately.
Assignee: law → netdemonz
In comment 57, Brian referenced two pages at CodeGuru that discussed how Developer Studio handles this sort of thing. If you have any Visual C++ from 6.0 through 7.1, you can try the following to see if you like this particular style of solution. Copy msdev.com from <VSDir>\Common\MSDev98\Bin (or devenv.com from <VSDir>\Common7\IDE) to your Mozilla directory. Rename the new copy Mozilla.com (or MozillaFirebird.com if using Mozilla Firebird). This works because the .com file uses its own name to determine the .exe to run. As long as start menu links and such point to the .exe rather than the .com, they'll work just like they always have. From the command line, if you don't specify the extension, Windows uses the .com version before the .exe version. Typing Mozilla /? dumps out the command line options. Typing Mozilla http://www.mozilla.org/ launches the browser at that page. Note: It's not perfect. On my machine, both .com versions occassionally stops printing out the options in the middle of -ProfileManager.
I'm considering whether to forget this bug altogether, and write a new bootloader to replace the current standard GUI bootloader Microsoft provides.
*** Bug 273892 has been marked as a duplicate of this bug. ***
As a workaround, use "firefox -h |more". It works perfectly, even if there's already a running instance. Ditto for Thunderbird. I don't have the Mozilla Suite.
*** This bug has been marked as a duplicate of 355889 ***
Status: NEW → RESOLVED
Closed: 25 years ago18 years ago
Resolution: --- → DUPLICATE
Isn't bug 355889 the duplicate?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: