Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small refactor to generalize config creation #1493

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Small refactor to generalize config creation #1493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 53 additions & 49 deletions src/contour/Config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,15 @@
return configHome("contour");
}

std::string createString(Config const& c)

Check warning on line 184 in src/contour/Config.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: parameter name 'c' is too short, expected at least 3 characters [readability-identifier-length]
```cpp
std::string createString(Config const& c)
                                       ^
```
{
return createString(c, YAMLConfigWriter());
}

std::string defaultConfigString()
{
const Config config {};
auto configString = YAMLConfigWriter().createString(config);
auto configString = createString(config);

auto logger = configLog;
logger()(configString);
Expand Down Expand Up @@ -1827,29 +1832,27 @@
logger()("Loading entry: {}, value {}", entry, where.value);
}

std::string YAMLConfigWriter::createString(Config const& c)
template <typename Writer>
std::string createString(Config const& c)

Check warning on line 1836 in src/contour/Config.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: parameter name 'c' is too short, expected at least 3 characters [readability-identifier-length]
```cpp
std::string createString(Config const& c)
                                       ^
```
{
auto doc = std::string {};
constexpr int OneOffset = 4;

Writer writer;
auto const process = [&](auto v) {
doc.append(format(addOffset(v.documentation, Offset::levels * OneOffset), v.value()));
doc.append(writer.process(v.documentation, v.value()));
};

auto const processWithDoc = [&](auto&& docString, auto... val) {
doc.append(fmt::format(
fmt::runtime(format(addOffset(std::string(docString.value), Offset::levels * OneOffset), val...)),
fmt::arg("comment", "#")));
doc.append(
fmt::format(fmt::runtime(writer.process(docString.value, val...)), fmt::arg("comment", "#")));
};

auto const processWordDelimiters = [&]() {
auto wordDelimiters = c.wordDelimiters.value();
wordDelimiters = std::regex_replace(wordDelimiters, std::regex("\\\\"), "\\$&"); /* \ -> \\ */
wordDelimiters = std::regex_replace(wordDelimiters, std::regex("\""), "\\$&"); /* " -> \" */
doc.append(fmt::format(
fmt::runtime(format(addOffset(c.wordDelimiters.documentation, Offset::levels * OneOffset),
wordDelimiters)),
fmt::arg("comment", "#")));
doc.append(
fmt::format(fmt::runtime(writer.process(documentation::WordDelimiters.value, wordDelimiters)),
fmt::arg("comment", "#")));
};

if (c.platformPlugin.value() == "")
Expand All @@ -1862,7 +1865,7 @@
}

// inside renderer:
scoped([&]() {
writer.scoped([&]() {
doc.append("renderer: \n");
process(c.renderingBackend);
process(c.textureAtlasHashtableSlots);
Expand All @@ -1886,7 +1889,7 @@
// inside images:
doc.append("\nimages: \n");

scoped([&]() {
writer.scoped([&]() {
process(c.sixelScrolling);
process(c.maxImageColorRegisters);
process(c.maxImageSize);
Expand All @@ -1895,12 +1898,12 @@
// inside profiles:
doc.append(fmt::format(fmt::runtime(c.profiles.documentation), fmt::arg("comment", "#")));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};

Check warning on line 1901 in src/contour/Config.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: variable name '_' is too short, expected at least 3 characters [readability-identifier-length]
```cpp
        const auto _ = typename Writer::Offset {};
                   ^
```
for (auto&& [name, entry]: c.profiles.value())
{
doc.append(fmt::format(" {}: \n", name));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
christianparpart marked this conversation as resolved.
Show resolved Hide resolved
process(entry.shell);
process(entry.ssh);

Expand All @@ -1925,27 +1928,27 @@

process(entry.margins);
// history: section
doc.append(addOffset("history:\n", Offset::levels * OneOffset));
doc.append(Writer::addOffset("history:\n", Writer::Offset::levels * Writer::OneOffset));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.maxHistoryLineCount);
process(entry.autoScrollOnUpdate);
process(entry.historyScrollMultiplier);
}

// scrollbar: section
doc.append(addOffset("scrollbar:\n", Offset::levels * OneOffset));
doc.append(Writer::addOffset("scrollbar:\n", Writer::Offset::levels * Writer::OneOffset));
;
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.scrollbarPosition);
process(entry.hideScrollbarInAltScreen);
}

// mouse: section
doc.append(addOffset("mouse:\n", Offset::levels * OneOffset));
doc.append(Writer::addOffset("mouse:\n", Writer::Offset::levels * Writer::OneOffset));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.mouseHideWhileTyping);
}

Expand All @@ -1963,7 +1966,7 @@
"permissions:\n" },
0);
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.changeFont);
process(entry.captureBuffer);
process(entry.displayHostWritableStatusLine);
Expand All @@ -1978,40 +1981,40 @@
process(entry.modalCursorScrollOff);

// status_line
doc.append(addOffset("\n"
"status_line:\n",
Offset::levels * OneOffset));
doc.append(Writer::addOffset("\n"
"status_line:\n",
Writer::Offset::levels * Writer::OneOffset));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.initialStatusDisplayType);
process(entry.statusDisplayPosition);
process(entry.syncWindowTitleWithHostWritableStatusDisplay);

doc.append(addOffset("indicator:\n", Offset::levels * OneOffset));
doc.append(Writer::addOffset("indicator:\n", Writer::Offset::levels * Writer::OneOffset));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.indicatorStatusLineLeft);
process(entry.indicatorStatusLineMiddle);
process(entry.indicatorStatusLineRight);
}
}

doc.append(addOffset("\n"
"background:\n",
Offset::levels * OneOffset));
doc.append(Writer::addOffset("\n"
"background:\n",
Writer::Offset::levels * Writer::OneOffset));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.backgroundOpacity);
process(entry.backgroundBlur);
}

process(entry.colors);

doc.append(addOffset("\n"
"hyperlink_decoration:\n",
Offset::levels * OneOffset));
doc.append(Writer::addOffset("\n"
"hyperlink_decoration:\n",
Writer::Offset::levels * Writer::OneOffset));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
process(entry.hyperlinkDecorationNormal);
process(entry.hyperlinkDecorationHover);
}
Expand All @@ -2020,19 +2023,20 @@
}

doc.append(fmt::format(fmt::runtime(c.colorschemes.documentation), fmt::arg("comment", "#")));
scoped([&]() {
writer.scoped([&]() {
for (auto&& [name, entry]: c.colorschemes.value())
{
doc.append(fmt::format(" {}: \n", name));

{
const auto _ = Offset {};
doc.append(fmt::format(fmt::runtime(addOffset("{comment} Default colors\n"
"default:\n",
Offset::levels * OneOffset)),
fmt::arg("comment", "#")));
const auto _ = typename Writer::Offset {};
doc.append(
fmt::format(fmt::runtime(Writer::addOffset("{comment} Default colors\n"
"default:\n",
Writer::Offset::levels * Writer::OneOffset)),
fmt::arg("comment", "#")));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};
processWithDoc(
documentation::DefaultColors, entry.defaultBackground, entry.defaultForeground);

Expand Down Expand Up @@ -2155,22 +2159,22 @@
entry.dimColor(7));
}

doc.append(addOffset("", Offset::levels * OneOffset));
doc.append(Writer::addOffset("", Writer::Offset::levels * Writer::OneOffset));
}
}
});

doc.append(fmt::format(fmt::runtime(c.inputMappings.documentation), fmt::arg("comment", "#")));
{
const auto _ = Offset {};
const auto _ = typename Writer::Offset {};

Check warning on line 2169 in src/contour/Config.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: variable name '_' is too short, expected at least 3 characters [readability-identifier-length]
```cpp
        const auto _ = typename Writer::Offset {};
                   ^
```
for (auto&& entry: c.inputMappings.value().keyMappings)
doc.append(addOffset(format(entry), Offset::levels * OneOffset));
doc.append(Writer::addOffset(writer.format(entry), Writer::Offset::levels * Writer::OneOffset));

for (auto&& entry: c.inputMappings.value().charMappings)
doc.append(addOffset(format(entry), Offset::levels * OneOffset));
doc.append(Writer::addOffset(writer.format(entry), Writer::Offset::levels * Writer::OneOffset));

for (auto&& entry: c.inputMappings.value().mouseMappings)
doc.append(addOffset(format(entry), Offset::levels * OneOffset));
doc.append(Writer::addOffset(writer.format(entry), Writer::Offset::levels * Writer::OneOffset));
}
return doc;
}
Expand Down
41 changes: 33 additions & 8 deletions src/contour/Config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include <regex>
#include <set>
#include <string>
#include <string_view>
#include <system_error>
#include <type_traits>
#include <unordered_map>
Expand Down Expand Up @@ -884,13 +885,9 @@
actions::Action action);
};

struct YAMLConfigWriter
struct Writer
{
std::string static addOffset(std::string const& doc, size_t off)
{
auto offset = std::string(off, ' ');
return std::regex_replace(doc, std::regex(".+\n"), offset + "$&");
}
virtual ~Writer() = default;

Check warning on line 890 in src/contour/Config.h

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: class 'Writer' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
```cpp
ct Writer
   ^
```

struct Offset
{
Expand All @@ -906,8 +903,6 @@
lambda();
}

std::string createString(Config const& c);

template <typename T>
auto format(T v)
{
Expand All @@ -919,6 +914,36 @@
{
return fmt::format(fmt::runtime(doc), format(args)..., fmt::arg("comment", "#"));
}
};

template <typename T>
std::string createString(Config const& c, T)
{
return createString<T>(c);

Check warning on line 922 in src/contour/Config.h

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: parameter name 'c' is too short, expected at least 3 characters [readability-identifier-length]
```cpp
st& c, T)
    ^
```

Check warning on line 922 in src/contour/Config.h

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: all parameters should be named in a function [readability-named-parameter]

```suggestion
st& c, T /*unused*/)
```
}

struct YAMLConfigWriter: Writer
{

static constexpr int OneOffset = 4;
using Writer::format;
std::string static addOffset(std::string const& doc, size_t off)
{
auto offset = std::string(off, ' ');
return std::regex_replace(doc, std::regex(".+\n"), offset + "$&");
}

template <typename T>
std::string process(std::string doc, T val)
{

Check warning on line 938 in src/contour/Config.h

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: the parameter 'doc' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
```cpp
doc, T val)
^
```
return format(addOffset(doc, Offset::levels * OneOffset), val);

Check warning on line 939 in src/contour/Config.h

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: performing an implicit widening conversion to type 'size_t' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]
```cpp
 val)
                                                 ^
```
<details>
<summary>Additional context</summary>

**src/contour/Config.h:938:** make conversion explicit to silence this warning
```cpp
 val)
                                                 ^
```
**src/contour/Config.h:938:** perform multiplication in a wider type
```cpp
 val)
                                                 ^
```

</details>
}

template <typename... T>
std::string process(std::string doc, T... val)

Check warning on line 943 in src/contour/Config.h

View workflow job for this annotation

GitHub Actions / clang-tidy-review

clang-tidy 

warning: the parameter 'doc' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
```cpp
ename... T>
                                                ^
```
{
return format(addOffset(doc, Offset::levels * OneOffset), val...);
}

[[nodiscard]] std::string format(KeyInputMapping v)
{
Expand Down
Loading