Skip to content

Commit 0ef21e2

Browse files
eseilereseiler
authored
[MISC] Add quotes to strings and paths (#224)
* [MISC] Add quotes to string and path options * [MISC] Also remove period after default message * [TEST] Add API patches
1 parent fb9552d commit 0ef21e2

15 files changed

+699
-73
lines changed

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ OPTIONS
129129
130130
Eating Numbers
131131
-i, --int (signed 32 bit integer)
132-
Desc. Default: 0.
132+
Desc. Default: 0
133133
134134
Common options
135135
-h, --help
@@ -144,7 +144,7 @@ OPTIONS
144144
Export the help page information. Value must be one of [html, man,
145145
ctd, cwl].
146146
--version-check (bool)
147-
Whether to check for the newest app version. Default: true.
147+
Whether to check for the newest app version. Default: true
148148
149149
VERSION
150150
Last update:

‎include/sharg/detail/format_base.hpp‎

Lines changed: 91 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class format_base
137137
*
138138
* \details Special characters considered are `"`, `\`, `&`, `<` and `>`.
139139
*/
140-
std::string escape_special_xml_chars(std::string const & original)
140+
static std::string escape_special_xml_chars(std::string const & original)
141141
{
142142
std::string escaped;
143143
escaped.reserve(original.size()); // will be at least as long
@@ -183,6 +183,62 @@ class format_base
183183

184184
return tmp;
185185
}
186+
187+
/*!\brief Returns the default message for the help page.
188+
* \tparam option_type The type of the option.
189+
* \tparam default_type The type of the default value.
190+
* \param[in] option The option to get the default message for.
191+
* \param[in] value The default value to get the default message for.
192+
* \returns The default message for the help page (" Default: <default-value>. ").
193+
* \details
194+
* `value` is either `config.default_message`, or the same as `option`.
195+
* If the `option_type` is a std::string or std::filesystem::path, the value is quoted.
196+
* If the `option_type` is a container of std::string or std::filesystem::path, each individual value is quoted;
197+
* if a `config.default_message` is provided, it will not be quoted.
198+
*/
199+
template <typename option_type, typename default_type>
200+
static std::string get_default_message(option_type const & SHARG_DOXYGEN_ONLY(option), default_type const & value)
201+
{
202+
static_assert(std::same_as<option_type, default_type> || std::same_as<default_type, std::string>);
203+
204+
std::stringstream message{};
205+
message << " Default: ";
206+
207+
if constexpr (detail::is_container_option<option_type>)
208+
{
209+
// If we have a list of strings, we want to quote each string.
210+
if constexpr (std::same_as<std::ranges::range_value_t<default_type>, std::string>)
211+
{
212+
auto view = std::views::transform(value,
213+
[](auto const & val)
214+
{
215+
return std::quoted(val);
216+
});
217+
message << detail::to_string(view);
218+
}
219+
else // Otherwise we just print the list or the default_message without quotes.
220+
{
221+
message << detail::to_string(value);
222+
}
223+
}
224+
else
225+
{
226+
static constexpr bool option_is_string = std::same_as<option_type, std::string>;
227+
static constexpr bool option_is_path = std::same_as<option_type, std::filesystem::path>;
228+
static constexpr bool value_is_string = std::same_as<default_type, std::string>;
229+
230+
// Quote: std::string (from value + default_message), and std::filesystem::path (default_message).
231+
// std::filesystem::path is quoted by the STL's operator<< in detail::to_string.
232+
static constexpr bool needs_string_quote = option_is_string || (option_is_path && value_is_string);
233+
234+
if constexpr (needs_string_quote)
235+
message << std::quoted(value);
236+
else
237+
message << detail::to_string(value);
238+
}
239+
240+
return message.str();
241+
}
186242
};
187243

188244
/*!\brief The format that contains all helper functions needed in all formats for
@@ -227,12 +283,14 @@ class format_help_base : public format_base
227283
{
228284
std::string id = prep_id_for_help(config.short_id, config.long_id) + " " + option_type_and_list_info(value);
229285
std::string info{config.description};
286+
230287
if (config.default_message.empty())
231-
info += ((config.required) ? std::string{" "} : detail::to_string(" Default: ", value, ". "));
288+
info += ((config.required) ? std::string{} : get_default_message(value, value));
232289
else
233-
info += detail::to_string(" Default: ", config.default_message, ". ");
290+
info += get_default_message(value, config.default_message);
234291

235-
info += config.validator.get_help_page_message();
292+
if (auto const & validator_message = config.validator.get_help_page_message(); !validator_message.empty())
293+
info += ". " + validator_message;
236294

237295
store_help_page_element(
238296
[this, id, info]()
@@ -262,20 +320,41 @@ class format_help_base : public format_base
262320
template <typename option_type, typename validator_t>
263321
void add_positional_option(option_type & value, config<validator_t> const & config)
264322
{
323+
// a list at the end may be empty and thus have a default value
324+
auto positional_default_message = [&value]() -> std::string
325+
{
326+
if constexpr (detail::is_container_option<option_type>)
327+
{
328+
return get_default_message(value, value);
329+
}
330+
else
331+
{
332+
(void)value; // Silence unused variable warning.
333+
return {};
334+
}
335+
};
336+
337+
auto positional_validator_message = [&config]() -> std::string
338+
{
339+
if (auto const & validator_message = config.validator.get_help_page_message(); !validator_message.empty())
340+
return ". " + validator_message;
341+
else
342+
return {};
343+
};
344+
265345
positional_option_calls.push_back(
266-
[this, &value, description = config.description, validator = config.validator]()
346+
[this,
347+
&value,
348+
default_message = positional_default_message(),
349+
validator_message = positional_validator_message(),
350+
description = config.description]()
267351
{
268352
++positional_option_count;
269353
derived_t().print_list_item(detail::to_string("\\fBARGUMENT-",
270354
positional_option_count,
271355
"\\fP ",
272356
option_type_and_list_info(value)),
273-
description +
274-
// a list at the end may be empty and thus have a default value
275-
((detail::is_container_option<option_type>)
276-
? detail::to_string(" Default: ", value, ". ")
277-
: std::string{" "})
278-
+ validator.get_help_page_message());
357+
description + default_message + validator_message);
279358
});
280359
}
281360

@@ -343,7 +422,7 @@ class format_help_base : public format_base
343422
+ detail::supported_exports + ".");
344423
if (version_check_dev_decision == update_notifications::on)
345424
derived_t().print_list_item("\\fB--version-check\\fP (bool)",
346-
"Whether to check for the newest app version. Default: true.");
425+
"Whether to check for the newest app version. Default: true");
347426

348427
if (!meta.examples.empty())
349428
{

‎include/sharg/detail/format_tdl.hpp‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,14 @@ class format_tdl : format_base
170170
void add_option(option_type & value, config<validator_t> const & config)
171171
{
172172
auto description = config.description;
173-
description += (config.required ? std::string{" "} : detail::to_string(" Default: ", value, ". "));
174-
description += config.validator.get_help_page_message();
173+
174+
if (config.default_message.empty())
175+
description += ((config.required) ? std::string{} : get_default_message(value, value));
176+
else
177+
description += get_default_message(value, config.default_message);
178+
179+
if (auto const & validator_message = config.validator.get_help_page_message(); !validator_message.empty())
180+
description += ". " + validator_message;
175181

176182
auto tags = std::set<std::string>{};
177183
if (config.required)
@@ -263,19 +269,37 @@ class format_tdl : format_base
263269
template <typename option_type, typename validator_t>
264270
void add_positional_option(option_type & value, config<validator_t> const & config)
265271
{
266-
std::string msg = config.validator.get_help_page_message();
272+
// a list at the end may be empty and thus have a default value
273+
auto positional_default_message = [&value]() -> std::string
274+
{
275+
if constexpr (detail::is_container_option<option_type>)
276+
{
277+
return get_default_message(value, value);
278+
}
279+
else
280+
{
281+
(void)value; // Silence unused variable warning.
282+
return {};
283+
}
284+
};
285+
286+
auto positional_validator_message = [&config]() -> std::string
287+
{
288+
if (auto const & validator_message = config.validator.get_help_page_message(); !validator_message.empty())
289+
return ". " + validator_message;
290+
else
291+
return {};
292+
};
267293

268294
positional_option_calls.push_back(
269-
[this, &value, config, msg](std::string_view)
295+
[this,
296+
config,
297+
default_message = positional_default_message(),
298+
validator_message = positional_validator_message()](std::string_view)
270299
{
271300
auto id = "positional_" + std::to_string(positional_option_count);
272301
++positional_option_count;
273-
auto description =
274-
config.description +
275-
// a list at the end may be empty and thus have a default value
276-
((detail::is_container_option<option_type>) ? detail::to_string(" Default: ", value, ". ")
277-
: std::string{" "})
278-
+ msg;
302+
auto description = config.description + default_message + validator_message;
279303

280304
parameters.push_back(tdl::Node{
281305
.name = id,

‎include/sharg/detail/to_string.hpp‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ static std::string const supported_exports =
2828
"[html, man]";
2929
#endif
3030

31+
//!\brief Concept for views whose value type is ostreamable.
32+
template <typename container_t>
33+
concept is_ostreamable_view = std::ranges::view<container_t> && ostreamable<std::ranges::range_value_t<container_t>>;
34+
3135
/*!\brief Streams all parameters via std::ostringstream and returns a concatenated string.
3236
* \ingroup misc
3337
* \tparam value_types Must be sharg::ostreamable (stream << value).
@@ -42,7 +46,11 @@ std::string to_string(value_types &&... values)
4246

4347
auto print = [&stream](auto && val)
4448
{
45-
if constexpr (is_container_option<std::remove_cvref_t<decltype(val)>>)
49+
using value_t = std::remove_cvref_t<decltype(val)>;
50+
51+
// When passing a `std::vector<std::string> | std::views::transform(...)` which returns `std::quoted(str)` for
52+
// each element, the `std::quoted`'s return value does not model a range, but is ostreamable.
53+
if constexpr (is_container_option<value_t> || is_ostreamable_view<value_t>)
4654
{
4755
if (val.empty())
4856
{
@@ -58,8 +66,7 @@ std::string to_string(value_types &&... values)
5866
stream << ']';
5967
}
6068
}
61-
else if constexpr (std::is_same_v<std::remove_cvref_t<decltype(val)>, int8_t>
62-
|| std::is_same_v<std::remove_cvref_t<decltype(val)>, uint8_t>)
69+
else if constexpr (std::is_same_v<value_t, int8_t> || std::is_same_v<value_t, uint8_t>)
6370
{
6471
stream << static_cast<int16_t>(val);
6572
}

‎test/api_stability/1.1.1/0001-API-Update-TDL.patch‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
From 724ad34dea87caf2dde5f19af51d7b5527acf3a2 Mon Sep 17 00:00:00 2001
2+
From: Simon Gene Gottlieb <[email protected]>
3+
Date: Thu, 5 Oct 2023 13:43:30 +0200
4+
Subject: [PATCH 1/2] [API] Update TDL
5+
6+
---
7+
test/unit/detail/format_ctd_test.cpp | 2 +-
8+
test/unit/detail/format_cwl_test.cpp | 22 +++++++++++-----------
9+
2 files changed, 12 insertions(+), 12 deletions(-)
10+
11+
diff --git a/test/unit/detail/format_ctd_test.cpp b/test/unit/detail/format_ctd_test.cpp
12+
index fa6a5b6..352acaa 100644
13+
--- a/test/unit/detail/format_ctd_test.cpp
14+
+++ b/test/unit/detail/format_ctd_test.cpp
15+
@@ -110,7 +110,7 @@ TEST_F(format_ctd_test, empty_information)
16+
// Create the dummy parser.
17+
sharg::parser parser{"default", argv.size(), argv.data()};
18+
parser.info.date = "December 01, 1994";
19+
- parser.info.version = "1.1.1";
20+
+ parser.info.version = "1.1.2-rc.1";
21+
parser.info.man_page_title = "default_man_page_title";
22+
parser.info.short_description = "A short description here.";
23+
24+
diff --git a/test/unit/detail/format_cwl_test.cpp b/test/unit/detail/format_cwl_test.cpp
25+
index d2bb143..5591a88 100644
26+
--- a/test/unit/detail/format_cwl_test.cpp
27+
+++ b/test/unit/detail/format_cwl_test.cpp
28+
@@ -20,12 +20,12 @@ TEST(format_cwl_test, empty_information)
29+
parser.info.man_page_title = "default_man_page_title";
30+
parser.info.short_description = "A short description here.";
31+
32+
- std::string expected_short = "inputs:\n"
33+
- " {}\n"
34+
- "outputs:\n"
35+
- " {}\n"
36+
- "label: default\n"
37+
+ std::string expected_short = "label: default\n"
38+
"doc: \"\"\n"
39+
+ "inputs:\n"
40+
+ " []\n"
41+
+ "outputs:\n"
42+
+ " []\n"
43+
"cwlVersion: v1.2\n"
44+
"class: CommandLineTool\n"
45+
"baseCommand:\n"
46+
@@ -75,7 +75,9 @@ TEST(format_cwl_test, full_information)
47+
parser.info.examples.push_back("example");
48+
parser.info.examples.push_back("example2");
49+
50+
- std::string expected_short = "inputs:\n"
51+
+ std::string expected_short = "label: default\n"
52+
+ "doc: \"description\\ndescription2\\n\"\n"
53+
+ "inputs:\n"
54+
" int:\n"
55+
" doc: \"this is a int option. Default: 5. \"\n"
56+
" type: long?\n"
57+
@@ -87,9 +89,7 @@ TEST(format_cwl_test, full_information)
58+
" inputBinding:\n"
59+
" prefix: --jint\n"
60+
"outputs:\n"
61+
- " {}\n"
62+
- "label: default\n"
63+
- "doc: \"description\\ndescription2\\n\"\n"
64+
+ " []\n"
65+
"cwlVersion: v1.2\n"
66+
"class: CommandLineTool\n"
67+
"baseCommand:\n"
68+
@@ -193,6 +193,8 @@ TEST(format_cwl_test, subparser)
69+
sub_parser.info.examples.push_back("example2");
70+
71+
std::string expected_short =
72+
+ "label: default-index\n"
73+
+ "doc: \"\"\n"
74+
"inputs:\n"
75+
" int:\n"
76+
" doc: \"this is a int option. Default: 5. \"\n"
77+
@@ -251,8 +253,6 @@ TEST(format_cwl_test, subparser)
78+
" type: Directory?\n"
79+
" outputBinding:\n"
80+
" glob: $(inputs.path05)\n"
81+
- "label: default-index\n"
82+
- "doc: \"\"\n"
83+
"cwlVersion: v1.2\n"
84+
"class: CommandLineTool\n"
85+
"baseCommand:\n"
86+
--
87+
2.43.0
88+

0 commit comments

Comments
 (0)