Re: [libvirt] [PATCH 17/30] remote: Remove virConnectPtr from error/errorf
On 04/12/2010 09:03 AM, Dave Allan wrote: +remoteError(VIR_ERR_INVALID_ARG, "%s", +_("transport methods unix, ssh and ext are not supported " + "under Windows")); >>> >>> I see why you broke this line, to fit 80 columns, but that can impact >>> grep-ability of the original message. Is there any policy on this? >>> >> >> I'm not aware of any policy for this, but we have many error messages >> split into multiple lines already. > > I agree that it impacts grep-ability in the code, but very long lines > impact the readability of the code, so in general, I'm in favor of > splitting lines. Good to know; thanks for the answers. Meanwhile, I've seen this paradigm in some other projects (glibc, coreutils, ...), to at least reduce the number of splits due to long messages: remoteError(VIR_ERR_INVALID_ARG, "%s", _("\ transport methods unix, ssh and ext are not supported under Windows")); By using the \-newline combo, and starting the message at column 1, you've bought yourself quite a bit of line length, without any change to the resulting message either on screen or in the .pot file. But I'm not bothered enough by the current situation to spend any of my time reformatting existing messages to a new format, so for now, I'm happy leaving things as they are. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/30] remote: Remove virConnectPtr from error/errorf
On Fri, Apr 09, 2010 at 02:03:09AM +0200, Matthias Bolte wrote: > 2010/4/5 Eric Blake : > > On 04/04/2010 11:36 AM, Matthias Bolte wrote: > >> Also unify error/errorf to remoteError and update cfg.mk accordingly. > > > >> +++ b/src/remote/remote_driver.c > >> @@ -239,11 +239,9 @@ static int remoteAuthSASL (virConnectPtr conn, struct > >> private_data *priv, int in > >> static int remoteAuthPolkit (virConnectPtr conn, struct private_data > >> *priv, int in_open, > >> virConnectAuthPtr auth); > >> #endif /* HAVE_POLKIT */ > >> -#define error(conn, code, info) \ > >> - virReportErrorHelper(conn, VIR_FROM_QEMU, code, __FILE__, \ > >> - __FUNCTION__, __LINE__, "%s", info) > >> -#define errorf(conn, code, ...) \ > >> - virReportErrorHelper(conn, VIR_FROM_QEMU, code, __FILE__, \ > >> + > >> +#define remoteError(code, ...) \ > >> + virReportErrorHelper(NULL, VIR_FROM_REMOTE, code, __FILE__, \ > >> __FUNCTION__, __LINE__, __VA_ARGS__) > > > > I like the renaming, especially since our use of the fixed-arg > > preprocessor macro error() was at odds with glibc's variadic function of > > the same name. > > > > ACK, and the rest of the patch is mechanical fallout. > > Thanks, pushed. > > >> @@ -825,8 +824,9 @@ doRemoteOpen (virConnectPtr conn, > >> case trans_unix: > >> case trans_ssh: > >> case trans_ext: > >> - error (conn, VIR_ERR_INVALID_ARG, > >> - _("transport methods unix, ssh and ext are not supported > >> under Windows")); > >> + remoteError(VIR_ERR_INVALID_ARG, "%s", > >> + _("transport methods unix, ssh and ext are not > >> supported " > >> + "under Windows")); > > > > I see why you broke this line, to fit 80 columns, but that can impact > > grep-ability of the original message. Is there any policy on this? > > > > I'm not aware of any policy for this, but we have many error messages > split into multiple lines already. I agree that it impacts grep-ability in the code, but very long lines impact the readability of the code, so in general, I'm in favor of splitting lines. Dave > Matthias > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/30] remote: Remove virConnectPtr from error/errorf
2010/4/5 Eric Blake : > On 04/04/2010 11:36 AM, Matthias Bolte wrote: >> Also unify error/errorf to remoteError and update cfg.mk accordingly. > >> +++ b/src/remote/remote_driver.c >> @@ -239,11 +239,9 @@ static int remoteAuthSASL (virConnectPtr conn, struct >> private_data *priv, int in >> static int remoteAuthPolkit (virConnectPtr conn, struct private_data *priv, >> int in_open, >> virConnectAuthPtr auth); >> #endif /* HAVE_POLKIT */ >> -#define error(conn, code, info) \ >> - virReportErrorHelper(conn, VIR_FROM_QEMU, code, __FILE__, \ >> - __FUNCTION__, __LINE__, "%s", info) >> -#define errorf(conn, code, ...) \ >> - virReportErrorHelper(conn, VIR_FROM_QEMU, code, __FILE__, \ >> + >> +#define remoteError(code, ...) \ >> + virReportErrorHelper(NULL, VIR_FROM_REMOTE, code, __FILE__, \ >> __FUNCTION__, __LINE__, __VA_ARGS__) > > I like the renaming, especially since our use of the fixed-arg > preprocessor macro error() was at odds with glibc's variadic function of > the same name. > > ACK, and the rest of the patch is mechanical fallout. Thanks, pushed. >> @@ -825,8 +824,9 @@ doRemoteOpen (virConnectPtr conn, >> case trans_unix: >> case trans_ssh: >> case trans_ext: >> - error (conn, VIR_ERR_INVALID_ARG, >> - _("transport methods unix, ssh and ext are not supported >> under Windows")); >> + remoteError(VIR_ERR_INVALID_ARG, "%s", >> + _("transport methods unix, ssh and ext are not >> supported " >> + "under Windows")); > > I see why you broke this line, to fit 80 columns, but that can impact > grep-ability of the original message. Is there any policy on this? > I'm not aware of any policy for this, but we have many error messages split into multiple lines already. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/30] remote: Remove virConnectPtr from error/errorf
On 04/04/2010 11:36 AM, Matthias Bolte wrote: > Also unify error/errorf to remoteError and update cfg.mk accordingly. > +++ b/src/remote/remote_driver.c > @@ -239,11 +239,9 @@ static int remoteAuthSASL (virConnectPtr conn, struct > private_data *priv, int in > static int remoteAuthPolkit (virConnectPtr conn, struct private_data *priv, > int in_open, > virConnectAuthPtr auth); > #endif /* HAVE_POLKIT */ > -#define error(conn, code, info) \ > -virReportErrorHelper(conn, VIR_FROM_QEMU, code, __FILE__, \ > - __FUNCTION__, __LINE__, "%s", info) > -#define errorf(conn, code, ...) \ > -virReportErrorHelper(conn, VIR_FROM_QEMU, code, __FILE__, \ > + > +#define remoteError(code, ...)\ > +virReportErrorHelper(NULL, VIR_FROM_REMOTE, code, __FILE__, \ > __FUNCTION__, __LINE__, __VA_ARGS__) I like the renaming, especially since our use of the fixed-arg preprocessor macro error() was at odds with glibc's variadic function of the same name. ACK, and the rest of the patch is mechanical fallout. > @@ -825,8 +824,9 @@ doRemoteOpen (virConnectPtr conn, > case trans_unix: > case trans_ssh: > case trans_ext: > -error (conn, VIR_ERR_INVALID_ARG, > - _("transport methods unix, ssh and ext are not supported > under Windows")); > +remoteError(VIR_ERR_INVALID_ARG, "%s", > +_("transport methods unix, ssh and ext are not supported > " > + "under Windows")); I see why you broke this line, to fit 80 columns, but that can impact grep-ability of the original message. Is there any policy on this? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list