Add missing constants and function for Memcached #3873
Conversation
|
Awesome work, @cracksalad, thank you so much! |
reference/memcached/constants.xml
Outdated
| <simpara>The <link xlink:href="&url.pecl.package;msgpack">MessagePack</link> serializer.</simpara> | ||
| <para> | ||
| This serializer is supported only if memcached is configured with | ||
| <literal>--enable-memcached-msgpack</literal> option and the | ||
| <literal>msgpack</literal> extension is loaded. | ||
| </para> |
There was a problem hiding this comment.
Just noticed that you sometimes use <simpara>, sometimes <para> and sometimes both in a <listitem> element. Please use always <simpara> there (unless <para> would be required, but I don't think this is the case here).
There was a problem hiding this comment.
@cmb69 When using
<simpara>foo</simpara>
<simpara>bar</simpara>there is no line break in between.
In this specific example it would look like this (which in this case would not be a problem, but other cases would look worse):
There was a problem hiding this comment.
Oh, indeed, for whatever reason a simpara inside a listitem is rendered as span:
Although DocBook says:
Processing expectations
Formatted as a displayed block.
Not sure what to do. Fixing PhD seems to be trivial, but might break some intentional rendering. @Girgias, @haszi, @jimwins, thoughts?
There was a problem hiding this comment.
I think <simpara> should be rendered as <p> and if that causes HTML layout problems when it is inside a <note>, <listitem>, or <entry>, we need to change the CSS styling so it looks how we want. Maybe we should make it <p class="simpara">.
(I would also be happy to see <simpara> just go away in favor of always using <para> because it seems rather pointless and probably some artifact of optimizing for old XML processing toolchains. But I haven't looked deeply at DocBook to see if there are places where you can only use <simpara> and not <para> that justify its existence.)
There was a problem hiding this comment.
https://tdg.docbook.org/tdg/5.2/para:
A para is a paragraph. Paragraphs in DocBook may contain almost all inlines and most block elements.
The latter doesn't fit well with HTML's <p> element (what is currently used to render <para>). In my opinion, we should use <simpara> whenever suitable (i.e. when containing only inline elements), and only use <para> where necessary.
There was a problem hiding this comment.
I can see 3 solutions to the problem here.
Mapping <simpara> to <p> except if it is in a <itemizedlist><listitem> or <orderedlist><listitem> sequence.
(this would cover the issue here where a <simpara> is used in a <listitem> in a <varlistentry>.
We convert those compact lists to <simplelist> tags.
We add a role="compact" on <itemizedlist>/<orderedlist> to change the processing inline (this might be annoying considering how PhD is implemented).
There was a problem hiding this comment.
I think <simplelist> wouldn't make sense semantically as it is supposed to be "An undecorated list of single words or short phrases".
The implementation of the other two options would take about the same amount of effort but I'm in favor of the role="compact" on <itemizedlist>/<orderedlist> elements as this shows intent in the documentation and can directly be controlled from there instead of the special casing hack that would only be known to those few of us who were part of this conversation. :-)
There was a problem hiding this comment.
Then let's do the role="compact" option. :)
There was a problem hiding this comment.
How to proceed with this PR: Should I wait for the role="compact" change to be in place or should I change the paras to simparas right now?
There was a problem hiding this comment.
I suggest to stick with you have for now. Changing to simpara would break the rendering, and waiting on role="compact" to be implemented might leave this important PR hanging around.
reference/memcached/constants.xml
Outdated
| <simpara>The <link xlink:href="&url.pecl.package;msgpack">MessagePack</link> serializer.</simpara> | ||
| <para> | ||
| This serializer is supported only if memcached is configured with | ||
| <literal>--enable-memcached-msgpack</literal> option and the | ||
| <literal>msgpack</literal> extension is loaded. | ||
| </para> |
There was a problem hiding this comment.
I can see 3 solutions to the problem here.
Mapping <simpara> to <p> except if it is in a <itemizedlist><listitem> or <orderedlist><listitem> sequence.
(this would cover the issue here where a <simpara> is used in a <listitem> in a <varlistentry>.
We convert those compact lists to <simplelist> tags.
We add a role="compact" on <itemizedlist>/<orderedlist> to change the processing inline (this might be annoying considering how PhD is implemented).
| <member><literal>Memcached::COMPRESSION_FASTLZ</literal> does not support compression levels.</member> | ||
| <member><literal>Memcached::COMPRESSION_ZSTD</literal>: minimum level is <literal>-22</literal> and maximum level is <literal>22</literal>.</member> | ||
| <member><literal>Memcached::COMPRESSION_ZLIB</literal>: minimum level is <literal>0</literal> and maximum level is <literal>9</literal>.</member> | ||
| </simplelist> | ||
| <para>Type: <type>int</type>, default: <literal>3</literal>.</para> |
There was a problem hiding this comment.
Just realizing that normally we put the type info in the <term> tag, but this is a follow-up.
There was a problem hiding this comment.
What about the default values?
(1) Should they be moved to the <term> tag as well or
(2) would you describe them in a simpara kind of as is or
(3) should we move them to the setOption-method doc, cause constants do not really have defaults, or
(4) should we restructure the "Predefined Constants" site to have different sections (one for the OPT_* constants, one for option values, one for RES_* constants etc.) and maybe use a table for the OPT_* constants (as is for DateTime) where we could add a column for the defaults
(5) more differently...
There was a problem hiding this comment.
I don't quite understand how a constant would have a default value. However, I agree with @Girgias that we can do that in a follow-up PR; this one is already very comprehensive.
There was a problem hiding this comment.
I don't quite understand how a constant would have a default value.
It doesn't! But the constant is used as the "name" of an option which has a default. I think it is really important to document those defaults, but I am not sure where. Since you can think of the documentation of those constants as the documentation of the configuration options, it seems to be the right place.
There was a problem hiding this comment.
The standard way to markup constant lists is with <variablelist>, but you can have multiple ones and even add a <title> tag to describe the grouping, look at streams/constants.xml for a rewrite that I recently did.
Tables are not great as they might not fit the width of the screen (e.g. mobile) and at present nearly all constant lists don't use tables.
There was a problem hiding this comment.
Please revert the markup changes for functions where the documentation did not change (effectively revert Remove unnecessary para tag wrapping variablelist tag for Memcached methods). Yes, the markup is not what I consider ideal, but no need to give unnecessary work to translations.
But there were exactly 42 🎉 changed files, now there are only 5 left 😢 |
|
@cracksalad, we try to distinguish between fixes/improvements to the documentation which affect all translations (beside EN, there are about 10 of these active), and changes which are likely only relevant for EN (such as whitespace changes, typo fixes etc.) The latter can be marked as The (now reverted) markup changes would have affected all translations; much work for a small gain. Still, we may consider to do these changes for EN in a separate PR; that is then still something which translators may skip at their own decision. |
|
@cmb69 I was just joking. 42 simply felt right 😂 So this |
Ah, got it. :)
|
|
Also please rebase, as it now conflicts because of #4028 |
This PR...
References: