Repository navigation
Conversation
`dump-config` reports only the interpreter and the paths imports resolve from, so there is no way to see what any other setting resolved to. Someone debugging a config that appears to be ignored has nothing to inspect. This adds the end-to-end test for a `--full` flag that prints the rest of the resolved settings, ahead of implementing it. The config under test sets a path-scoped override, two error severities written out of alphabetical order, and a misspelled key, so the test pins down the sort order and the reporting of unrecognized keys as well as the overall shape of the output.
Users are pointed at `dump-config` to work out why a config is not behaving the way they expect, but it only reports the interpreter and the paths imports resolve from. Every other setting is invisible, and a key Pyrefly did not recognize is dropped silently, which is exactly the case the command is reached for. `--full` prints the rest of the resolved configuration in the indented, prose-labelled style the command already uses. Default output is untouched, so the existing behaviour and its tests still hold. The rendering lives in `pyrefly_config` next to `structured_import_lookup_path` because several `ConfigBase` settings are crate-private and unreadable from the command, which only prints the lines it is handed. Settings the import lookup path already covers are left out so that `--full` adds information rather than repeating it, and the error severities are sorted on the way out because they are stored in a `HashMap`.
The import resolution guide is where users are sent to debug configuration problems, so the new flag is described alongside the command it belongs to.
Use the config's serializer so dump-config --full picks up new settings without maintaining a separate list. Omit settings already shown by the command and sort nested tables for stable output.
|
✅ This PR meets all requirements from our pull request checklist. Optional recommendations:
Note: this comment updates automatically after changes to the PR. Updates may take up to half an hour. |
Author
Contributor
|
This pull request has been imported. If you are a Meta employee, you can view this in D128841161. (Because this pull request was imported automatically, there will not be any future comments.) |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
dump-config --fullto show resolved configuration settings beyond the interpreter and import paths, making it easier to understand why a setting is not taking effect.Fixes #1483.
This replaces #4732 and addresses @rchen152’s review feedback.
Approach
The command serializes the resolved configuration using the existing
tomlcrate, removes settings already covered by the interpreter and import-path output, and prints the remaining settings as TOML.The output includes error severity overrides, path-scoped
sub-configoverrides, and unrecognized configuration keys and their values. Tables are sorted recursively for stable output, while array order is preserved.Default command output and
--max-filesbehavior remain unchanged. Unset options and some settings at their default values are omitted according to the existing serialization rules.Addressing the earlier review
In the review of #4732, @rchen152 pointed out that
base_settingsduplicated the list of configuration fields and would need updating whenever a setting was added.Following that suggestion, I replaced the manual field list and custom rendering with TOML serialization. New serializable settings are now included automatically, without maintaining a second list.
Why
--fullinstead of--verbose?The issue suggests
--verbose, but the existing global-v/--verboseflag controls logging verbosity.--fullkeeps configuration output separate from logging.Test Plan
cargo test -p pyrefly_config --lib --locked: 271 tests passed.test/config.mdandtest/interpreter.md: 25 tests passed.Validation ran on macOS. Linux and Windows validation remains for CI.
AI usage disclosure
I used Claude for parts of the original investigation and implementation, update the tests and documentation, and run validation.