Print validate and metaschema paths relative to the working directory - #853
Conversation
…tory Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
There was a problem hiding this comment.
1 issue found across 125 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="test/validate/fail_yaml_multi_one_verbose.clitest">
<violation number="1" location="test/validate/fail_yaml_multi_one_verbose.clitest:26">
P3: The verbose `validate` output is now inconsistent: the ok/matches/fail lines print instance and schema paths relative to the working directory, but the adjacent "Interpreting input as YAML multi-document" line (src/input.h:403) still prints the absolute path via canonical.generic_string(). Since the PR's stated goal is to print paths relative to the working directory, confirm whether this message should also be made relative for consistent output in a single run.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| 2> ok: [CWD]/instance.yaml (entry #1) | ||
| 2> matches [CWD]/schema.json | ||
| 2> fail: [CWD]/instance.yaml (entry #2) | ||
| 2> ok: instance.yaml (entry #1) |
There was a problem hiding this comment.
P3: The verbose validate output is now inconsistent: the ok/matches/fail lines print instance and schema paths relative to the working directory, but the adjacent "Interpreting input as YAML multi-document" line (src/input.h:403) still prints the absolute path via canonical.generic_string(). Since the PR's stated goal is to print paths relative to the working directory, confirm whether this message should also be made relative for consistent output in a single run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/validate/fail_yaml_multi_one_verbose.clitest, line 26:
<comment>The verbose `validate` output is now inconsistent: the ok/matches/fail lines print instance and schema paths relative to the working directory, but the adjacent "Interpreting input as YAML multi-document" line (src/input.h:403) still prints the absolute path via canonical.generic_string(). Since the PR's stated goal is to print paths relative to the working directory, confirm whether this message should also be made relative for consistent output in a single run.</comment>
<file context>
@@ -23,9 +23,9 @@ REPLACE $CWD WITH '[CWD]' IN result_0.txt
-2> ok: [CWD]/instance.yaml (entry #1)
-2> matches [CWD]/schema.json
-2> fail: [CWD]/instance.yaml (entry #2)
+2> ok: instance.yaml (entry #1)
+2> matches schema.json
+2> fail: instance.yaml (entry #2)
</file context>
🤖 Augment PR SummarySummary: This PR makes user-facing paths from Changes:
Technical Notes:
🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
1 issue found across 206 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/command_validate.cc">
<violation number="1" location="src/command_validate.cc:273">
P3: `weakly_canonical` resolves symlinks, so the path printed in the `ok:`/`fail:`/`matches` (verbose) and benchmark output is the resolved target, not the path the user typed. For a schema or instance reached through a symlink (e.g. `../schema.json` -> a file outside the working tree, or a path whose ancestor is a symlink), the display path becomes an absolute or `..`-heavy resolved location instead of the argument the user supplied, which is what `relative_path_string`'s comment (`// Per instance output names paths the way the user did`) promises. Confirm this is intended, or derive the display path lexically (`std::filesystem::absolute` + `lexically_normal`) instead of resolving symlinks when you only need a working-directory-relative display name.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Re-trigger cubic
| : std::filesystem::path(schema_path)}; | ||
| const auto schema_resolution_base{ | ||
| schema_from_stdin ? stdin_path() : std::filesystem::path(schema_path)}; | ||
| const auto schema_display_path{ |
There was a problem hiding this comment.
P3: weakly_canonical resolves symlinks, so the path printed in the ok:/fail:/matches (verbose) and benchmark output is the resolved target, not the path the user typed. For a schema or instance reached through a symlink (e.g. ../schema.json -> a file outside the working tree, or a path whose ancestor is a symlink), the display path becomes an absolute or ..-heavy resolved location instead of the argument the user supplied, which is what relative_path_string's comment (// Per instance output names paths the way the user did) promises. Confirm this is intended, or derive the display path lexically (std::filesystem::absolute + lexically_normal) instead of resolving symlinks when you only need a working-directory-relative display name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/command_validate.cc, line 273:
<comment>`weakly_canonical` resolves symlinks, so the path printed in the `ok:`/`fail:`/`matches` (verbose) and benchmark output is the resolved target, not the path the user typed. For a schema or instance reached through a symlink (e.g. `../schema.json` -> a file outside the working tree, or a path whose ancestor is a symlink), the display path becomes an absolute or `..`-heavy resolved location instead of the argument the user supplied, which is what `relative_path_string`'s comment (`// Per instance output names paths the way the user did`) promises. Confirm this is intended, or derive the display path lexically (`std::filesystem::absolute` + `lexically_normal`) instead of resolving symlinks when you only need a working-directory-relative display name.</comment>
<file context>
@@ -270,6 +270,10 @@ auto sourcemeta::jsonschema::validate(const sourcemeta::core::Options &options)
: std::filesystem::path(schema_path)};
const auto schema_resolution_base{
schema_from_stdin ? stdin_path() : std::filesystem::path(schema_path)};
+ const auto schema_display_path{
+ schema_from_stdin
+ ? stdin_path()
</file context>
Signed-off-by: Juan Cruz Viotti jv@jviotti.com