mirror of
https://github.com/RGBCube/serenity
synced 2025-05-14 08:14:58 +00:00
Meta+Utilities: Make pre-commit checks significantly less verbose
When markdown-check is built, it outputs hundreds of lines of "ignoring this and that link because reasons". This is extremely not helpful when trying to figure out exactly which check failed on your commit. Also remove the timing numbers from lint-ci.sh These are just noise and also don't help to figure out which pre-commit check failed. Ideally the output on fail should be "[OK]: Check A" for all the passing checks and "[FAIL] Check N" with the required context for the failed check.
This commit is contained in:
parent
b284e525f3
commit
5028223c37
2 changed files with 22 additions and 19 deletions
|
@ -34,8 +34,7 @@ for cmd in \
|
||||||
Meta/lint-prettier.sh \
|
Meta/lint-prettier.sh \
|
||||||
Meta/lint-python.sh \
|
Meta/lint-python.sh \
|
||||||
Meta/lint-shell-scripts.sh; do
|
Meta/lint-shell-scripts.sh; do
|
||||||
echo "Running ${cmd}"
|
if "${cmd}" "$@"; then
|
||||||
if time "${cmd}" "$@"; then
|
|
||||||
echo -e "[${GREEN}OK${NC}]: ${cmd}"
|
echo -e "[${GREEN}OK${NC}]: ${cmd}"
|
||||||
else
|
else
|
||||||
echo -e "[${RED}FAIL${NC}]: ${cmd}"
|
echo -e "[${RED}FAIL${NC}]: ${cmd}"
|
||||||
|
@ -44,8 +43,7 @@ for cmd in \
|
||||||
done
|
done
|
||||||
|
|
||||||
if [ -x ./Build/lagom/bin/IPCMagicLinter ]; then
|
if [ -x ./Build/lagom/bin/IPCMagicLinter ]; then
|
||||||
echo "Running IPCMagicLinter"
|
if { git ls-files '*.ipc' | xargs ./Build/lagom/bin/IPCMagicLinter; }; then
|
||||||
if time { git ls-files '*.ipc' | xargs ./Build/lagom/bin/IPCMagicLinter; }; then
|
|
||||||
echo -e "[${GREEN}OK${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"
|
echo -e "[${GREEN}OK${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"
|
||||||
else
|
else
|
||||||
echo -e "[${RED}FAIL${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"
|
echo -e "[${RED}FAIL${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"
|
||||||
|
@ -55,8 +53,7 @@ else
|
||||||
echo -e "[${GREEN}SKIP${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"
|
echo -e "[${GREEN}SKIP${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo "Running Meta/lint-clang-format.sh"
|
if Meta/lint-clang-format.sh --overwrite-inplace "$@" && git diff --exit-code; then
|
||||||
if time Meta/lint-clang-format.sh --overwrite-inplace "$@" && git diff --exit-code; then
|
|
||||||
echo -e "[${GREEN}OK${NC}]: Meta/lint-clang-format.sh"
|
echo -e "[${GREEN}OK${NC}]: Meta/lint-clang-format.sh"
|
||||||
else
|
else
|
||||||
echo -e "[${RED}FAIL${NC}]: Meta/lint-clang-format.sh"
|
echo -e "[${RED}FAIL${NC}]: Meta/lint-clang-format.sh"
|
||||||
|
@ -70,8 +67,7 @@ fi
|
||||||
# when Ports/ files have changed and only invoke lint-ports.py when needed.
|
# when Ports/ files have changed and only invoke lint-ports.py when needed.
|
||||||
#
|
#
|
||||||
if [ "$ports" = true ]; then
|
if [ "$ports" = true ]; then
|
||||||
echo "Running Meta/lint-ports.py"
|
if Meta/lint-ports.py; then
|
||||||
if time Meta/lint-ports.py; then
|
|
||||||
echo -e "[${GREEN}OK${NC}]: Meta/lint-ports.py"
|
echo -e "[${GREEN}OK${NC}]: Meta/lint-ports.py"
|
||||||
else
|
else
|
||||||
echo -e "[${RED}FAIL${NC}]: Meta/lint-ports.py"
|
echo -e "[${RED}FAIL${NC}]: Meta/lint-ports.py"
|
||||||
|
|
|
@ -72,7 +72,7 @@ class MarkdownLinkage final : Markdown::Visitor {
|
||||||
public:
|
public:
|
||||||
~MarkdownLinkage() = default;
|
~MarkdownLinkage() = default;
|
||||||
|
|
||||||
static MarkdownLinkage analyze(Markdown::Document const&);
|
static MarkdownLinkage analyze(Markdown::Document const&, bool verbose);
|
||||||
|
|
||||||
bool has_anchor(DeprecatedString const& anchor) const { return m_anchors.contains(anchor); }
|
bool has_anchor(DeprecatedString const& anchor) const { return m_anchors.contains(anchor); }
|
||||||
HashTable<DeprecatedString> const& anchors() const { return m_anchors; }
|
HashTable<DeprecatedString> const& anchors() const { return m_anchors; }
|
||||||
|
@ -80,7 +80,8 @@ public:
|
||||||
Vector<FileLink> const& file_links() const { return m_file_links; }
|
Vector<FileLink> const& file_links() const { return m_file_links; }
|
||||||
|
|
||||||
private:
|
private:
|
||||||
MarkdownLinkage()
|
MarkdownLinkage(bool verbose)
|
||||||
|
: m_verbose(verbose)
|
||||||
{
|
{
|
||||||
auto const* source_directory = getenv("SERENITY_SOURCE_DIR");
|
auto const* source_directory = getenv("SERENITY_SOURCE_DIR");
|
||||||
if (source_directory != nullptr) {
|
if (source_directory != nullptr) {
|
||||||
|
@ -96,13 +97,14 @@ private:
|
||||||
HashTable<DeprecatedString> m_anchors;
|
HashTable<DeprecatedString> m_anchors;
|
||||||
Vector<FileLink> m_file_links;
|
Vector<FileLink> m_file_links;
|
||||||
bool m_has_invalid_link { false };
|
bool m_has_invalid_link { false };
|
||||||
|
bool m_verbose { false };
|
||||||
|
|
||||||
DeprecatedString m_serenity_source_directory;
|
DeprecatedString m_serenity_source_directory;
|
||||||
};
|
};
|
||||||
|
|
||||||
MarkdownLinkage MarkdownLinkage::analyze(Markdown::Document const& document)
|
MarkdownLinkage MarkdownLinkage::analyze(Markdown::Document const& document, bool verbose)
|
||||||
{
|
{
|
||||||
MarkdownLinkage linkage;
|
MarkdownLinkage linkage(verbose);
|
||||||
|
|
||||||
document.walk(linkage);
|
document.walk(linkage);
|
||||||
|
|
||||||
|
@ -184,7 +186,8 @@ RecursionDecision MarkdownLinkage::visit(Markdown::Text::LinkNode const& link_no
|
||||||
auto url = URL::create_with_url_or_path(href);
|
auto url = URL::create_with_url_or_path(href);
|
||||||
if (url.is_valid()) {
|
if (url.is_valid()) {
|
||||||
if (url.scheme() == "https" || url.scheme() == "http") {
|
if (url.scheme() == "https" || url.scheme() == "http") {
|
||||||
outln("Not checking external link {}", href);
|
if (m_verbose)
|
||||||
|
outln("Not checking external link {}", href);
|
||||||
return RecursionDecision::Recurse;
|
return RecursionDecision::Recurse;
|
||||||
}
|
}
|
||||||
if (url.scheme() == "help") {
|
if (url.scheme() == "help") {
|
||||||
|
@ -224,7 +227,7 @@ RecursionDecision MarkdownLinkage::visit(Markdown::Text::LinkNode const& link_no
|
||||||
warnln("Binary link named '{}' is not allowed, binary links must be called 'Open'. Linked binary: {}", link_text, href);
|
warnln("Binary link named '{}' is not allowed, binary links must be called 'Open'. Linked binary: {}", link_text, href);
|
||||||
m_has_invalid_link = true;
|
m_has_invalid_link = true;
|
||||||
}
|
}
|
||||||
} else {
|
} else if (m_verbose) {
|
||||||
outln("Not checking local link {}", href);
|
outln("Not checking local link {}", href);
|
||||||
}
|
}
|
||||||
return RecursionDecision::Recurse;
|
return RecursionDecision::Recurse;
|
||||||
|
@ -287,13 +290,16 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
|
||||||
Core::ArgsParser args_parser;
|
Core::ArgsParser args_parser;
|
||||||
Vector<StringView> file_paths;
|
Vector<StringView> file_paths;
|
||||||
bool output_link_graph { false };
|
bool output_link_graph { false };
|
||||||
|
bool verbose_output { false };
|
||||||
StringView base_path = "/"sv;
|
StringView base_path = "/"sv;
|
||||||
args_parser.add_positional_argument(file_paths, "Path to markdown files to read and parse", "paths", Core::ArgsParser::Required::Yes);
|
args_parser.add_positional_argument(file_paths, "Path to markdown files to read and parse", "paths", Core::ArgsParser::Required::Yes);
|
||||||
args_parser.add_option(base_path, "System base path (default: \"/\")", "base", 'b', "path");
|
args_parser.add_option(base_path, "System base path (default: \"/\")", "base", 'b', "path");
|
||||||
args_parser.add_option(output_link_graph, "Output a page link graph into \"manpage-links.gv\". The recommended tool to process this graph is `fdp`.", "link-graph", 'g');
|
args_parser.add_option(output_link_graph, "Output a page link graph into \"manpage-links.gv\". The recommended tool to process this graph is `fdp`.", "link-graph", 'g');
|
||||||
|
args_parser.add_option(verbose_output, "Print extra information about skipped links", "verbose", 'v');
|
||||||
args_parser.parse(arguments);
|
args_parser.parse(arguments);
|
||||||
|
|
||||||
outln("Reading and parsing Markdown files ...");
|
if (verbose_output)
|
||||||
|
outln("Reading and parsing Markdown files ...");
|
||||||
HashMap<String, MarkdownLinkage> files;
|
HashMap<String, MarkdownLinkage> files;
|
||||||
for (auto path : file_paths) {
|
for (auto path : file_paths) {
|
||||||
auto file_or_error = Core::File::open(path, Core::File::OpenMode::Read);
|
auto file_or_error = Core::File::open(path, Core::File::OpenMode::Read);
|
||||||
|
@ -319,10 +325,11 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
|
||||||
// Since this should never happen anyway, fail early.
|
// Since this should never happen anyway, fail early.
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
files.set(TRY(FileSystem::real_path(path)), MarkdownLinkage::analyze(*document));
|
files.set(TRY(FileSystem::real_path(path)), MarkdownLinkage::analyze(*document, verbose_output));
|
||||||
}
|
}
|
||||||
|
|
||||||
outln("Checking links ...");
|
if (verbose_output)
|
||||||
|
outln("Checking links ...");
|
||||||
bool any_problems = false;
|
bool any_problems = false;
|
||||||
for (auto const& file_item : files) {
|
for (auto const& file_item : files) {
|
||||||
if (file_item.value.has_invalid_link()) {
|
if (file_item.value.has_invalid_link()) {
|
||||||
|
@ -425,8 +432,8 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
|
||||||
if (any_problems) {
|
if (any_problems) {
|
||||||
outln("Done. Some errors were encountered, please check above log.");
|
outln("Done. Some errors were encountered, please check above log.");
|
||||||
return 1;
|
return 1;
|
||||||
} else {
|
} else if (verbose_output) {
|
||||||
outln("Done. No problems detected.");
|
outln("Done. No problems detected.");
|
||||||
return 0;
|
|
||||||
}
|
}
|
||||||
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue