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
