1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-04 09:43:08 +02:00

Correctly display current values for indexed options in help.

The current value for an indexed option was always for the first index, e.g. pg1-path. This is likely legacy from before indexing was added (and faithfully copied over from Perl, apparently).

Fix this by enumerating the current values in the option help and displaying <multi> in the option list when more than one value exists.
This commit is contained in:
David Steele 2024-10-05 09:41:50 +03:00 committed by GitHub
parent 047e3d0ed9
commit b3ca2e3482
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 133 additions and 9 deletions

View File

@ -49,6 +49,17 @@
<p>Refresh web-id token for each <proper>S3</proper> authentication.</p>
</release-item>
<release-item>
<github-pull-request id="2453"/>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="david.christensen"/>
</release-item-contributor-list>
<p>Correctly display current values for indexed options in help.</p>
</release-item>
<release-item>
<github-pull-request id="2403"/>

View File

@ -157,10 +157,10 @@ helpRenderText(
}
/***********************************************************************************************************************************
Helper function for helpRender() to output values as strings
Helper functions for helpRender() to output values as strings
***********************************************************************************************************************************/
static String *
helpRenderValue(const ConfigOption optionId, const unsigned int optionIdx)
helpRenderValueIdx(const ConfigOption optionId, const unsigned int optionIdx)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(ENUM, optionId);
@ -169,7 +169,7 @@ helpRenderValue(const ConfigOption optionId, const unsigned int optionIdx)
String *result = NULL;
if (cfgOptionIdxSource(optionId, 0) != cfgSourceDefault)
if (cfgOptionIdxSource(optionId, optionIdx) != cfgSourceDefault)
{
result = strNew();
@ -224,6 +224,42 @@ helpRenderValue(const ConfigOption optionId, const unsigned int optionIdx)
FUNCTION_TEST_RETURN(STRING, result);
}
static String *
helpRenderValue(const ConfigOption optionId)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(ENUM, optionId);
FUNCTION_TEST_END();
String *result = helpRenderValueIdx(optionId, 0);
if (cfgOptionGroup(optionId))
{
MEM_CONTEXT_TEMP_BEGIN()
{
for (unsigned int optionIdx = 1; optionIdx < cfgOptionGroupIdxTotal(cfgOptionGroupId(optionId)); optionIdx++)
{
const String *const value = helpRenderValueIdx(optionId, optionIdx);
if (!strEq(result, value))
{
MEM_CONTEXT_PRIOR_BEGIN()
{
strFree(result);
result = strNewZ("<multi>");
}
MEM_CONTEXT_PRIOR_END();
break;
}
}
}
MEM_CONTEXT_TEMP_END();
}
FUNCTION_TEST_RETURN(STRING, result);
}
/***********************************************************************************************************************************
Render help to a string
***********************************************************************************************************************************/
@ -476,7 +512,7 @@ helpRender(const Buffer *const helpData)
// Output current and default values if they exist
const String *const defaultValue = cfgOptionDefault(optionId);
const String *const value = helpRenderValue(optionId, 0);
const String *const value = helpRenderValue(optionId);
if (value != NULL || defaultValue != NULL)
{
@ -544,14 +580,35 @@ helpRender(const Buffer *const helpData)
// Output current and default values if they exist
const String *const defaultValue = cfgOptionDefault(option.id);
const String *const value = helpRenderValue(option.id, 0);
const String *const value = helpRenderValue(option.id);
if (value != NULL || defaultValue != NULL)
{
strCat(result, LF_STR);
if (value != NULL)
strCatFmt(result, "current: %s\n", cfgParseOptionSecure(option.id) ? "<redacted>" : strZ(value));
{
strCatZ(result, "current:");
if (cfgParseOptionSecure(option.id))
strCatZ(result, " <redacted>\n");
else if (!strEqZ(value, "<multi>"))
strCatFmt(result, " %s\n", strZ(value));
else
{
const unsigned int groupId = cfgOptionGroupId(option.id);
strCatChr(result, '\n');
for (unsigned int optionIdx = 0; optionIdx < cfgOptionGroupIdxTotal(groupId); optionIdx++)
{
const String *const value = helpRenderValueIdx(option.id, optionIdx);
if (value != NULL)
strCatFmt(result, " %s: %s\n", cfgOptionGroupName(groupId, optionIdx), strZ(value));
}
}
}
if (defaultValue != NULL)
strCatFmt(result, "default: %s\n", strZ(defaultValue));

View File

@ -308,14 +308,14 @@ testRun(void)
" --repo-cipher-pass repository cipher passphrase\n"
" [current=<redacted>]\n"
" --repo-cipher-type cipher used to encrypt the repository\n"
" [current=aes-256-cbc, default=none]\n"
" [current=<multi>, default=none]\n"
" --repo-gcs-bucket GCS repository bucket\n"
" --repo-gcs-endpoint GCS repository endpoint\n"
" [default=storage.googleapis.com]\n"
" --repo-gcs-key GCS repository key\n"
" --repo-gcs-key-type GCS repository key type [default=service]\n"
" --repo-host repository host when operating remotely\n"
" [current=backup.example.net]\n"
" [current=<multi>]\n"
" --repo-host-ca-file repository host certificate authority file\n"
" --repo-host-ca-path repository host certificate authority path\n"
" --repo-host-cert-file repository host certificate file\n"
@ -383,7 +383,8 @@ testRun(void)
hrnCfgArgRawZ(argList, cfgOptBufferSize, "32768");
hrnCfgArgRawZ(argList, cfgOptRepoCipherType, "aes-256-cbc");
hrnCfgEnvRawZ(cfgOptRepoCipherPass, TEST_CIPHER_PASS);
hrnCfgArgRawZ(argList, cfgOptRepoHost, "backup.example.net");
hrnCfgArgKeyRawZ(argList, cfgOptRepoHost, 1, "backup.example.net");
hrnCfgArgKeyRawZ(argList, cfgOptRepoHost, 2, "dr.test.org");
hrnCfgArgRawZ(argList, cfgOptLinkMap, "/link1=/dest1");
hrnCfgArgRawZ(argList, cfgOptLinkMap, "/link2=/dest2");
hrnCfgArgRawZ(argList, cfgOptDbInclude, "db1");
@ -584,6 +585,61 @@ testRun(void)
strLstAddZ(argList, "repo-retention-archive");
TEST_RESULT_VOID(testCfgLoad(argList), "help for backup command, repo-retention-archive option");
TEST_RESULT_STR_Z(helpRender(helpData), optionHelp, "check admonition text");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("multiple current values (with one missing)");
optionHelp = zNewFmt(
"%s - 'restore' command - 'repo-host' option help\n"
"\n"
"Repository host when operating remotely.\n"
"\n"
"When backing up and archiving to a locally mounted filesystem this setting is\n"
"not required.\n"
"\n"
"current:\n"
" repo1: backup.example.net\n"
" repo3: dr.test.org\n"
"\n"
"deprecated name: backup-host\n",
helpVersion);
argList = strLstNew();
strLstAddZ(argList, "/path/to/pgbackrest");
hrnCfgArgKeyRawZ(argList, cfgOptRepoHost, 1, "backup.example.net");
hrnCfgArgKeyRawZ(argList, cfgOptRepoHost, 3, "dr.test.org");
strLstAddZ(argList, "help");
strLstAddZ(argList, "restore");
strLstAddZ(argList, "repo-host");
TEST_RESULT_VOID(testCfgLoad(argList), "help for restore command, repo-host option");
TEST_RESULT_STR_Z(helpRender(helpData), optionHelp, "check text");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("multiple current values (with one unset)");
optionHelp = zNewFmt(
"%s - 'restore' command - 'repo-host' option help\n"
"\n"
"Repository host when operating remotely.\n"
"\n"
"When backing up and archiving to a locally mounted filesystem this setting is\n"
"not required.\n"
"\n"
"current:\n"
" repo3: dr.test.org\n"
"\n"
"deprecated name: backup-host\n",
helpVersion);
argList = strLstNew();
strLstAddZ(argList, "/path/to/pgbackrest");
hrnCfgArgKeyRawZ(argList, cfgOptRepoType, 1, "s3");
hrnCfgArgKeyRawZ(argList, cfgOptRepoHost, 3, "dr.test.org");
strLstAddZ(argList, "help");
strLstAddZ(argList, "restore");
strLstAddZ(argList, "repo-host");
TEST_RESULT_VOID(testCfgLoad(argList), "help for restore command, repo-host option");
TEST_RESULT_STR_Z(helpRender(helpData), optionHelp, "check text");
}
// *****************************************************************************************************************************