Re: Add hint message for check_log_destination()
On Thu, 13 Jul 2023 at 16:19, Masahiko Sawada wrote: > On Tue, Jul 11, 2023 at 10:24 AM Japin Li wrote: >> >> >> On Mon, 10 Jul 2023 at 14:23, Masahiko Sawada wrote: >> > On Mon, Jul 10, 2023 at 2:07 PM Kyotaro Horiguchi >> > wrote: >> >> >> >> At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li wrote >> >> in >> >> > >> >> > On Sat, 08 Jul 2023 at 12:48, Michael Paquier >> >> > wrote: >> >> > > On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote: >> >> > >> + appendStringInfoString(&errhint, "\"stderr\""); >> >> > >> +#ifdef HAVE_SYSLOG >> >> > >> + appendStringInfoString(&errhint, ", \"syslog\""); >> >> > >> +#endif >> >> > >> +#ifdef WIN32 >> >> > >> + appendStringInfoString(&errhint, ", >> >> > >> \"eventlog\""); >> >> > >> +#endif >> >> > >> + appendStringInfoString(&errhint, ", \"csvlog\", >> >> > >> and \"jsonlog\""); >> >> > > >> >> > > Hmm. Is that OK as a translatable string? >> > >> > It seems okay to me but needs to be checked. >> > >> >> > >> >> > >> >> > Sorry for the late reply! I'm not sure. How can I know whether it is >> >> > translatable? >> >> >> >> At the very least, we can't generate comma-separated lists >> >> programatically because punctuation marks vary across languages. >> >> >> >> One potential approach could involve defining the message for every >> >> potential combination, in full length. >> > >> > Don't we generate a comma-separated list for an error hint of an enum >> > parameter? For example, to generate the following error hint: >> > >> > =# alter system set client_min_messages = 'aaa'; >> > ERROR: invalid value for parameter "client_min_messages": "aaa" >> > HINT: Available values: debug5, debug4, debug3, debug2, debug1, log, >> > notice, warning, error. >> > >> > we use the comma-separated generated by config_enum_get_options() and >> > do ereport() like: >> > >> > ereport(elevel, >> > (errcode(ERRCODE_INVALID_PARAMETER_VALUE), >> >errmsg("invalid value for parameter \"%s\": \"%s\"", >> > name, value), >> >hintmsg ? errhint("%s", _(hintmsg)) : 0)); >> >> > IMO log_destination is a string GUC parameter but its value is the >> > list of enums. So it makes sense to me to add a hint message like what >> > we do for enum parameters in case where the user mistypes a wrong >> > value. I'm not sure why the proposed patch needs to quote the usable >> > values, though. >> >> I borrowed the description from pg_settings extra_desc. In my first patch, >> I used the hint message line enum parameter, however, since it might be a >> combination of multiple log destinations, so, I update the patch using >> extra_desc. > > I agree to use description from pg_settings extra_desc, but it seems > to be better not to quote each available value like we do for enum > parameter cases. That is, the hint message would be like: > > =# alter system set log_destination to 'xxx'; > ERROR: invalid value for parameter "log_destination": "xxx" > DETAIL: Unrecognized key word: "xxx". > HINT: Valid values are combinations of stderr, syslog, csvlog, and jsonlog. > Agreed. Fixed as per your suggestions. -- Regrads, Japin Li >From 3f709effe14de81b84bbc441aa5884ece0a757da Mon Sep 17 00:00:00 2001 From: Japin Li Date: Fri, 14 Jul 2023 09:26:01 +0800 Subject: [PATCH v4 1/1] Add hint message for check_log_destination --- src/backend/utils/error/elog.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 5898100acb..dccbabf40a 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2228,8 +2228,22 @@ check_log_destination(char **newval, void **extra, GucSource source) #endif else { + StringInfoData errhint; + + initStringInfo(&errhint); + appendStringInfoString(&errhint, "stderr"); +#ifdef HAVE_SYSLOG + appendStringInfoString(&errhint, ", syslog"); +#endif +#ifdef WIN32 + appendStringInfoString(&errhint, ", eventlog"); +#endif + appendStringInfoString(&errhint, ", csvlog, and jsonlog"); + GUC_check_errdetail("Unrecognized key word: \"%s\".", tok); + GUC_check_errhint("Valid values are combinations of %s.", errhint.data); pfree(rawstring); + pfree(errhint.data); list_free(elemlist); return false; } -- 2.41.0
Re: Add hint message for check_log_destination()
On Tue, Jul 11, 2023 at 10:24 AM Japin Li wrote: > > > On Mon, 10 Jul 2023 at 14:23, Masahiko Sawada wrote: > > On Mon, Jul 10, 2023 at 2:07 PM Kyotaro Horiguchi > > wrote: > >> > >> At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li wrote in > >> > > >> > On Sat, 08 Jul 2023 at 12:48, Michael Paquier > >> > wrote: > >> > > On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote: > >> > >> + appendStringInfoString(&errhint, "\"stderr\""); > >> > >> +#ifdef HAVE_SYSLOG > >> > >> + appendStringInfoString(&errhint, ", \"syslog\""); > >> > >> +#endif > >> > >> +#ifdef WIN32 > >> > >> + appendStringInfoString(&errhint, ", \"eventlog\""); > >> > >> +#endif > >> > >> + appendStringInfoString(&errhint, ", \"csvlog\", > >> > >> and \"jsonlog\""); > >> > > > >> > > Hmm. Is that OK as a translatable string? > > > > It seems okay to me but needs to be checked. > > > >> > > >> > > >> > Sorry for the late reply! I'm not sure. How can I know whether it is > >> > translatable? > >> > >> At the very least, we can't generate comma-separated lists > >> programatically because punctuation marks vary across languages. > >> > >> One potential approach could involve defining the message for every > >> potential combination, in full length. > > > > Don't we generate a comma-separated list for an error hint of an enum > > parameter? For example, to generate the following error hint: > > > > =# alter system set client_min_messages = 'aaa'; > > ERROR: invalid value for parameter "client_min_messages": "aaa" > > HINT: Available values: debug5, debug4, debug3, debug2, debug1, log, > > notice, warning, error. > > > > we use the comma-separated generated by config_enum_get_options() and > > do ereport() like: > > > > ereport(elevel, > > (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > >errmsg("invalid value for parameter \"%s\": \"%s\"", > > name, value), > >hintmsg ? errhint("%s", _(hintmsg)) : 0)); > > > IMO log_destination is a string GUC parameter but its value is the > > list of enums. So it makes sense to me to add a hint message like what > > we do for enum parameters in case where the user mistypes a wrong > > value. I'm not sure why the proposed patch needs to quote the usable > > values, though. > > I borrowed the description from pg_settings extra_desc. In my first patch, > I used the hint message line enum parameter, however, since it might be a > combination of multiple log destinations, so, I update the patch using > extra_desc. I agree to use description from pg_settings extra_desc, but it seems to be better not to quote each available value like we do for enum parameter cases. That is, the hint message would be like: =# alter system set log_destination to 'xxx'; ERROR: invalid value for parameter "log_destination": "xxx" DETAIL: Unrecognized key word: "xxx". HINT: Valid values are combinations of stderr, syslog, csvlog, and jsonlog. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Re: Add hint message for check_log_destination()
On Mon, 10 Jul 2023 at 14:23, Masahiko Sawada wrote: > On Mon, Jul 10, 2023 at 2:07 PM Kyotaro Horiguchi > wrote: >> >> At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li wrote in >> > >> > On Sat, 08 Jul 2023 at 12:48, Michael Paquier wrote: >> > > On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote: >> > >> + appendStringInfoString(&errhint, "\"stderr\""); >> > >> +#ifdef HAVE_SYSLOG >> > >> + appendStringInfoString(&errhint, ", \"syslog\""); >> > >> +#endif >> > >> +#ifdef WIN32 >> > >> + appendStringInfoString(&errhint, ", \"eventlog\""); >> > >> +#endif >> > >> + appendStringInfoString(&errhint, ", \"csvlog\", and >> > >> \"jsonlog\""); >> > > >> > > Hmm. Is that OK as a translatable string? > > It seems okay to me but needs to be checked. > >> > >> > >> > Sorry for the late reply! I'm not sure. How can I know whether it is >> > translatable? >> >> At the very least, we can't generate comma-separated lists >> programatically because punctuation marks vary across languages. >> >> One potential approach could involve defining the message for every >> potential combination, in full length. > > Don't we generate a comma-separated list for an error hint of an enum > parameter? For example, to generate the following error hint: > > =# alter system set client_min_messages = 'aaa'; > ERROR: invalid value for parameter "client_min_messages": "aaa" > HINT: Available values: debug5, debug4, debug3, debug2, debug1, log, > notice, warning, error. > > we use the comma-separated generated by config_enum_get_options() and > do ereport() like: > > ereport(elevel, > (errcode(ERRCODE_INVALID_PARAMETER_VALUE), >errmsg("invalid value for parameter \"%s\": \"%s\"", > name, value), >hintmsg ? errhint("%s", _(hintmsg)) : 0)); > IMO log_destination is a string GUC parameter but its value is the > list of enums. So it makes sense to me to add a hint message like what > we do for enum parameters in case where the user mistypes a wrong > value. I'm not sure why the proposed patch needs to quote the usable > values, though. I borrowed the description from pg_settings extra_desc. In my first patch, I used the hint message line enum parameter, however, since it might be a combination of multiple log destinations, so, I update the patch using extra_desc. -- Regrads, Japin Li.
Re: Add hint message for check_log_destination()
On Mon, Jul 10, 2023 at 2:07 PM Kyotaro Horiguchi wrote: > > At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li wrote in > > > > On Sat, 08 Jul 2023 at 12:48, Michael Paquier wrote: > > > On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote: > > >> + appendStringInfoString(&errhint, "\"stderr\""); > > >> +#ifdef HAVE_SYSLOG > > >> + appendStringInfoString(&errhint, ", \"syslog\""); > > >> +#endif > > >> +#ifdef WIN32 > > >> + appendStringInfoString(&errhint, ", \"eventlog\""); > > >> +#endif > > >> + appendStringInfoString(&errhint, ", \"csvlog\", and > > >> \"jsonlog\""); > > > > > > Hmm. Is that OK as a translatable string? It seems okay to me but needs to be checked. > > > > > > Sorry for the late reply! I'm not sure. How can I know whether it is > > translatable? > > At the very least, we can't generate comma-separated lists > programatically because punctuation marks vary across languages. > > One potential approach could involve defining the message for every > potential combination, in full length. Don't we generate a comma-separated list for an error hint of an enum parameter? For example, to generate the following error hint: =# alter system set client_min_messages = 'aaa'; ERROR: invalid value for parameter "client_min_messages": "aaa" HINT: Available values: debug5, debug4, debug3, debug2, debug1, log, notice, warning, error. we use the comma-separated generated by config_enum_get_options() and do ereport() like: ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid value for parameter \"%s\": \"%s\"", name, value), hintmsg ? errhint("%s", _(hintmsg)) : 0)); IMO log_destination is a string GUC parameter but its value is the list of enums. So it makes sense to me to add a hint message like what we do for enum parameters in case where the user mistypes a wrong value. I'm not sure why the proposed patch needs to quote the usable values, though. A similar type of GUC parameter is debug_io_direct. But I'm not sure we need a hint message for it too as it's a developer option. > On top of that, consider "csvlog" as an example, -- it > doesn't work as expected if logging_collector is off. Although this is > documented, we don't give any warnings at startup. This seems like a > bigger issue than the unusable keywords. (I don't mean to suggest to > fix this, as usual.) Yes, but I think it's a separate problem. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Re: Add hint message for check_log_destination()
On Mon, Jul 10, 2023 at 02:07:09PM +0900, Kyotaro Horiguchi wrote: > At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li wrote in >> Sorry for the late reply! I'm not sure. How can I know whether it is >> translatable? Per the documentation: https://www.postgresql.org/docs/devel/nls-programmer.html#NLS-GUIDELINES Now, if you want to look at the shape of the messages, you could also run something like a `make init-po` and look at the messages generated in a .pot file. > Honestly, I'm not sold on the idea that we need to exhaust ourselves > providing an exhaustive list of usable keywords for users here. I > believe that it is unlikely that these keywords will be used in > different combinations each time without looking at the > documentation. On top of that, consider "csvlog" as an example, -- it > doesn't work as expected if logging_collector is off. Although this is > documented, we don't give any warnings at startup. This seems like a > bigger issue than the unusable keywords. (I don't mean to suggest to > fix this, as usual.) > > In short, I think a simple message like '"xxx" cannot be used in this > build' should suffice for keywords defined but unusable, and we should > stick with "unknown" for the undefined ones. Which is roughly what the existing GUC_check_errdetail() does as well, but you indeed lose a bit of context because the option wanted is not built. I am not convinced that there is something to change here. -- Michael signature.asc Description: PGP signature
Re: Add hint message for check_log_destination()
At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li wrote in > > On Sat, 08 Jul 2023 at 12:48, Michael Paquier wrote: > > On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote: > >> + appendStringInfoString(&errhint, "\"stderr\""); > >> +#ifdef HAVE_SYSLOG > >> + appendStringInfoString(&errhint, ", \"syslog\""); > >> +#endif > >> +#ifdef WIN32 > >> + appendStringInfoString(&errhint, ", \"eventlog\""); > >> +#endif > >> + appendStringInfoString(&errhint, ", \"csvlog\", and > >> \"jsonlog\""); > > > > Hmm. Is that OK as a translatable string? > > > Sorry for the late reply! I'm not sure. How can I know whether it is > translatable? At the very least, we can't generate comma-separated lists programatically because punctuation marks vary across languages. One potential approach could involve defining the message for every potential combination, in full length. Honestly, I'm not sold on the idea that we need to exhaust ourselves providing an exhaustive list of usable keywords for users here. I believe that it is unlikely that these keywords will be used in different combinations each time without looking at the documentation. On top of that, consider "csvlog" as an example, -- it doesn't work as expected if logging_collector is off. Although this is documented, we don't give any warnings at startup. This seems like a bigger issue than the unusable keywords. (I don't mean to suggest to fix this, as usual.) In short, I think a simple message like '"xxx" cannot be used in this build' should suffice for keywords defined but unusable, and we should stick with "unknown" for the undefined ones. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Add hint message for check_log_destination()
On Sat, 08 Jul 2023 at 12:48, Michael Paquier wrote: > On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote: >> +appendStringInfoString(&errhint, "\"stderr\""); >> +#ifdef HAVE_SYSLOG >> +appendStringInfoString(&errhint, ", \"syslog\""); >> +#endif >> +#ifdef WIN32 >> +appendStringInfoString(&errhint, ", \"eventlog\""); >> +#endif >> +appendStringInfoString(&errhint, ", \"csvlog\", and >> \"jsonlog\""); > > Hmm. Is that OK as a translatable string? Sorry for the late reply! I'm not sure. How can I know whether it is translatable? -- Regrads, Japin Li.
Re: Add hint message for check_log_destination()
On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote: > + appendStringInfoString(&errhint, "\"stderr\""); > +#ifdef HAVE_SYSLOG > + appendStringInfoString(&errhint, ", \"syslog\""); > +#endif > +#ifdef WIN32 > + appendStringInfoString(&errhint, ", \"eventlog\""); > +#endif > + appendStringInfoString(&errhint, ", \"csvlog\", and > \"jsonlog\""); Hmm. Is that OK as a translatable string? -- Michael signature.asc Description: PGP signature
Re: Add hint message for check_log_destination()
On Fri, 07 Jul 2023 at 16:21, Masahiko Sawada wrote: > On Fri, Jul 7, 2023 at 4:53 PM Japin Li wrote: >> >> >> On Fri, 07 Jul 2023 at 14:46, jian he wrote: >> > On Fri, Jul 7, 2023 at 1:06 PM Japin Li wrote: >> >> >> >> >> >> Hi, hackers >> >> >> >> When I try to change log_destination using ALTER SYSTEM with the wrong >> >> value, >> >> it complains of the "Unrecognized key word" without available values. >> >> This >> >> patch tries to add a hint message that provides available values for >> >> log_destination. Any thoughts? > > +1 > > + appendStringInfo(&errhint, "\"stderr\""); > +#ifdef HAVE_SYSLOG > + appendStringInfo(&errhint, ", \"syslog\""); > +#endif > +#ifdef WIN32 > + appendStringInfo(&errhint, ", \"eventlog\""); > +#endif > + appendStringInfo(&errhint, ", \"csvlog\", and \"jsonlog\""); > > I think using appendStringInfoString() is a bit more natural and faster. > Thanks for your review! Fixed as per your suggession. >From e0338c5085e655f9a25f13cd5acea9a3110806fd Mon Sep 17 00:00:00 2001 From: Japin Li Date: Fri, 7 Jul 2023 15:48:53 +0800 Subject: [PATCH v3 1/1] Add hint message for check_log_destination --- src/backend/utils/error/elog.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 5898100acb..a7545dcbd9 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2228,8 +2228,22 @@ check_log_destination(char **newval, void **extra, GucSource source) #endif else { + StringInfoData errhint; + + initStringInfo(&errhint); + appendStringInfoString(&errhint, "\"stderr\""); +#ifdef HAVE_SYSLOG + appendStringInfoString(&errhint, ", \"syslog\""); +#endif +#ifdef WIN32 + appendStringInfoString(&errhint, ", \"eventlog\""); +#endif + appendStringInfoString(&errhint, ", \"csvlog\", and \"jsonlog\""); + GUC_check_errdetail("Unrecognized key word: \"%s\".", tok); + GUC_check_errhint("Valid values are combinations of %s.", errhint.data); pfree(rawstring); + pfree(errhint.data); list_free(elemlist); return false; } -- 2.25.1 -- Regrads, Japin Li.
Re: Add hint message for check_log_destination()
On Fri, Jul 7, 2023 at 4:53 PM Japin Li wrote: > > > On Fri, 07 Jul 2023 at 14:46, jian he wrote: > > On Fri, Jul 7, 2023 at 1:06 PM Japin Li wrote: > >> > >> > >> Hi, hackers > >> > >> When I try to change log_destination using ALTER SYSTEM with the wrong > >> value, > >> it complains of the "Unrecognized key word" without available values. This > >> patch tries to add a hint message that provides available values for > >> log_destination. Any thoughts? +1 + appendStringInfo(&errhint, "\"stderr\""); +#ifdef HAVE_SYSLOG + appendStringInfo(&errhint, ", \"syslog\""); +#endif +#ifdef WIN32 + appendStringInfo(&errhint, ", \"eventlog\""); +#endif + appendStringInfo(&errhint, ", \"csvlog\", and \"jsonlog\""); I think using appendStringInfoString() is a bit more natural and faster. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Re: Add hint message for check_log_destination()
On Fri, 07 Jul 2023 at 14:46, jian he wrote: > On Fri, Jul 7, 2023 at 1:06 PM Japin Li wrote: >> >> >> Hi, hackers >> >> When I try to change log_destination using ALTER SYSTEM with the wrong value, >> it complains of the "Unrecognized key word" without available values. This >> patch tries to add a hint message that provides available values for >> log_destination. Any thoughts? >> > > select * from pg_settings where name ~* 'log.*destin*' \gx > > short_desc | Sets the destination for server log output. > extra_desc | Valid values are combinations of "stderr", "syslog", > "csvlog", "jsonlog", and "eventlog", depending on the platform. > > you can just reuse extra_desc in the pg_settings (view) column ? Thanks for your review! Yeah, the description of extra_desc is more accurate. Updated. -- Regrads, Japin Li. >From 715f9811717c9d27f6b2f41db639b18e6ba58625 Mon Sep 17 00:00:00 2001 From: Japin Li Date: Fri, 7 Jul 2023 15:48:53 +0800 Subject: [PATCH v2 1/1] Add hint message for check_log_destination --- src/backend/utils/error/elog.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 5898100acb..a32f9613be 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2228,8 +2228,22 @@ check_log_destination(char **newval, void **extra, GucSource source) #endif else { + StringInfoData errhint; + + initStringInfo(&errhint); + appendStringInfo(&errhint, "\"stderr\""); +#ifdef HAVE_SYSLOG + appendStringInfo(&errhint, ", \"syslog\""); +#endif +#ifdef WIN32 + appendStringInfo(&errhint, ", \"eventlog\""); +#endif + appendStringInfo(&errhint, ", \"csvlog\", and \"jsonlog\""); + GUC_check_errdetail("Unrecognized key word: \"%s\".", tok); + GUC_check_errhint("Valid values are combinations of %s.", errhint.data); pfree(rawstring); + pfree(errhint.data); list_free(elemlist); return false; } -- 2.25.1
Re: Add hint message for check_log_destination()
On Fri, Jul 7, 2023 at 1:06 PM Japin Li wrote: > > > Hi, hackers > > When I try to change log_destination using ALTER SYSTEM with the wrong value, > it complains of the "Unrecognized key word" without available values. This > patch tries to add a hint message that provides available values for > log_destination. Any thoughts? > > -- > Regrads, > Japin Li. > select * from pg_settings where name ~* 'log.*destin*' \gx short_desc | Sets the destination for server log output. extra_desc | Valid values are combinations of "stderr", "syslog", "csvlog", "jsonlog", and "eventlog", depending on the platform. you can just reuse extra_desc in the pg_settings (view) column ?
Add hint message for check_log_destination()
Hi, hackers When I try to change log_destination using ALTER SYSTEM with the wrong value, it complains of the "Unrecognized key word" without available values. This patch tries to add a hint message that provides available values for log_destination. Any thoughts? -- Regrads, Japin Li. >From 7d6b50c9deaba576cdc28e170bff541f630415f9 Mon Sep 17 00:00:00 2001 From: Japin Li Date: Fri, 7 Jul 2023 12:59:09 +0800 Subject: [PATCH v1 1/1] Add hint message for log_destination --- src/backend/utils/error/elog.c | 13 + 1 file changed, 13 insertions(+) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 5898100acb..fdc6557a59 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2228,8 +2228,21 @@ check_log_destination(char **newval, void **extra, GucSource source) #endif else { + StringInfoData errhint; + + initStringInfo(&errhint); + appendStringInfo(&errhint, "stderr, csvlog, jsonlog"); +#ifdef HAVE_SYSLOG + appendStringInfo(&errhint, ", syslog"); +#endif +#ifdef WIN32 + appendStringInfo(&errhint, ", eventlog"); +#endif + GUC_check_errdetail("Unrecognized key word: \"%s\".", tok); + GUC_check_errhint("Available values: %s.", errhint.data); pfree(rawstring); + pfree(errhint.data); list_free(elemlist); return false; } -- 2.25.1