Message ID | 1306707770-14632-3-git-send-email-cerbere@gmail.com |
---|---|
State | New |
Headers | show |
On 29 May 2011 23:22, Alexandre Raymond <cerbere@gmail.com> wrote: > diff --git a/ui/cocoa.m b/ui/cocoa.m > index 1ff1ac6..e1312d3 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -872,7 +872,8 @@ int main (int argc, const char * argv[]) { > if (opt[1] == '-') { > opt++; > } > - if (!strcmp(opt, "-vnc") || > + if (!strcmp(opt, "-h") || !strcmp(opt, "-help") || > + !strcmp(opt, "-vnc") || > !strcmp(opt, "-nographic") || > !strcmp(opt, "-version") || > !strcmp(opt, "-curses")) { (1) presumably this doesn't work if you disable the display with "-display none" ? (2) it's pretty ugly and not very maintainable -- is there some restructuring possible to avoid having to hardcode information about qemu options into the ui code here? (It also doesn't catch other cases where qemu prints some information and exits immediately, like "-cpu ?".) -- PMM
I agree that this is not the best way to handle all cases not requiring a GUI. However, due to the current structure of the code, it was a simple way to cover a very common case without having to refactor the whole cocoa code. Alexandre On Sun, May 29, 2011 at 6:32 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 29 May 2011 23:22, Alexandre Raymond <cerbere@gmail.com> wrote: >> diff --git a/ui/cocoa.m b/ui/cocoa.m >> index 1ff1ac6..e1312d3 100644 >> --- a/ui/cocoa.m >> +++ b/ui/cocoa.m >> @@ -872,7 +872,8 @@ int main (int argc, const char * argv[]) { >> if (opt[1] == '-') { >> opt++; >> } >> - if (!strcmp(opt, "-vnc") || >> + if (!strcmp(opt, "-h") || !strcmp(opt, "-help") || >> + !strcmp(opt, "-vnc") || >> !strcmp(opt, "-nographic") || >> !strcmp(opt, "-version") || >> !strcmp(opt, "-curses")) { > > (1) presumably this doesn't work if you disable the display > with "-display none" ? > (2) it's pretty ugly and not very maintainable -- is there > some restructuring possible to avoid having to hardcode > information about qemu options into the ui code here? > > (It also doesn't catch other cases where qemu prints some > information and exits immediately, like "-cpu ?".) > > -- PMM >
Am 30.05.2011 um 00:22 schrieb Alexandre Raymond: > There was already a check in place to avoid displaying a window > in certain modes such as vnc, nographic or curses. > > Add a check for '-h' and '-help' to avoid displaying a window for a > split- > second before showing the usage information. > > Signed-off-by: Alexandre Raymond <cerbere@gmail.com> Thanks, applied to the cocoa branch. > --- > ui/cocoa.m | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/ui/cocoa.m b/ui/cocoa.m > index 1ff1ac6..e1312d3 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -872,7 +872,8 @@ int main (int argc, const char * argv[]) { > if (opt[1] == '-') { > opt++; > } > - if (!strcmp(opt, "-vnc") || > + if (!strcmp(opt, "-h") || !strcmp(opt, "-help") || > + !strcmp(opt, "-vnc") || > !strcmp(opt, "-nographic") || > !strcmp(opt, "-version") || > !strcmp(opt, "-curses")) { > -- > 1.7.5
Am 30.05.2011 um 00:32 schrieb Peter Maydell: > On 29 May 2011 23:22, Alexandre Raymond <cerbere@gmail.com> wrote: >> diff --git a/ui/cocoa.m b/ui/cocoa.m >> index 1ff1ac6..e1312d3 100644 >> --- a/ui/cocoa.m >> +++ b/ui/cocoa.m >> @@ -872,7 +872,8 @@ int main (int argc, const char * argv[]) { >> if (opt[1] == '-') { >> opt++; >> } >> - if (!strcmp(opt, "-vnc") || >> + if (!strcmp(opt, "-h") || !strcmp(opt, "-help") || >> + !strcmp(opt, "-vnc") || >> !strcmp(opt, "-nographic") || >> !strcmp(opt, "-version") || >> !strcmp(opt, "-curses")) { > > (1) presumably this doesn't work if you disable the display > with "-display none" ? I don't see how that would not work. It's just not handled specially here, so it will likely display a window - the former behavior of all these switches. > (2) it's pretty ugly and not very maintainable -- is there > some restructuring possible to avoid having to hardcode > information about qemu options into the ui code here? > > (It also doesn't catch other cases where qemu prints some > information and exits immediately, like "-cpu ?".) My saying! It's a general problem though: On my GNOME desktop I have some launchers for frequently used QEMU machines; it did occur that something changed in QEMU and nothing at all happened when double- clicking and I had to repeat the same in a terminal to find out why. Similar back when using a bundled Q.app on Mac OS X (i.e., a process that does not display a Terminal window). What I have asked for in the past is an override mechanism for error messages, so that at runtime we can detect properly whether we're running in console or window mode and choose to display a MessageBox on Windows, a modal sheet on Mac OS X, a BAlert or whatever a frontend author sees fit. Sequential fprintf(stderr, ...) is not really helpful for that use case. The added difficulty for Cocoa is that it needs to go through Objective-C (e.g., ui/cocoa.m). Since that is a larger task and a long-time open issue, I see no reason not to accept this patch as an interim solution. Andreas P.S. I haven't found any VNC viewer component either, to resort to a specialized virt-manager-like graphical interface process with child QEMU processes. So going down the VNC route as once under discussion would mean forking and maintaining a VNC client for a particular less- common platform, which I am not comfortable with, given the occasional protocol extensions contributed to QEMU.
On 2 June 2011 00:05, Andreas Färber <andreas.faerber@web.de> wrote: > Am 30.05.2011 um 00:32 schrieb Peter Maydell: >> (1) presumably this doesn't work if you disable the display >> with "-display none" ? > > I don't see how that would not work. It's just not handled specially here, > so it will likely display a window - the former behavior of all these > switches. Yes, that's my point -- this chunk of code is trying to implement "don't bring up the display window if we won't use it", and it doesn't work properly [ie does not suppress the window in all the cases where it should] because inevitably it lags behind the set of options in vl.c which don't bring up the display window. I don't particularly object to the patch being committed as a short term measure. -- PMM
diff --git a/ui/cocoa.m b/ui/cocoa.m index 1ff1ac6..e1312d3 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -872,7 +872,8 @@ int main (int argc, const char * argv[]) { if (opt[1] == '-') { opt++; } - if (!strcmp(opt, "-vnc") || + if (!strcmp(opt, "-h") || !strcmp(opt, "-help") || + !strcmp(opt, "-vnc") || !strcmp(opt, "-nographic") || !strcmp(opt, "-version") || !strcmp(opt, "-curses")) {
There was already a check in place to avoid displaying a window in certain modes such as vnc, nographic or curses. Add a check for '-h' and '-help' to avoid displaying a window for a split- second before showing the usage information. Signed-off-by: Alexandre Raymond <cerbere@gmail.com> --- ui/cocoa.m | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)