Skip to content

refactor: Unify pyRevit configuration handling#3450

Draft
ChrisCrosley wants to merge 10 commits into
pyrevitlabs:developfrom
ChrisCrosley:config-store-phase1
Draft

refactor: Unify pyRevit configuration handling#3450
ChrisCrosley wants to merge 10 commits into
pyrevitlabs:developfrom
ChrisCrosley:config-store-phase1

Conversation

@ChrisCrosley

@ChrisCrosley ChrisCrosley commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

pyRevit currently has three independent configuration readers for pyrevit_config.ini, each with its own parsing, defaults, and quirks that have drifted apart over time. Also, there is no sharing of parsed configs between them, so on load the C# loader parses and then each python engine also parses. This leads to the configs being read from disk once by the C# loader, then again for every startup script and smartbutton engine (at least 5 times for a basic pyRevit install):

  • the Python layer (pyrevit.coreutils.configparser),
  • the CLI library (pyRevitLabs.PyRevit),
  • the C# loader (pyRevitExtensionParser).

This PR starts with a clean port of @dosymep's #2482 ("refactor: Rewrite pyRevit configurations") onto current develop and reconciling it with everything that changed since that PR was opened, and proposes 2 additional phases of work to complete the feature.

Whats in this PR (Phase 1)

New pyRevitLabs.Configurations assembly family ported from #2482:

  • IConfigurationService / IConfiguration with attribute-bound typed POCO sections ([SectionName] / [KeyName]) for [core], [routes], [telemetry], and [environment], with defaults centralized on the sections instead of scattered across readers.
  • A pluggable INI backend (pyRevitLabs.Configurations.Ini) — the only backend ported (see below re: the dropped JSON/YAML backends).
  • xUnit test projects for the abstraction and the INI backend.

CLI migrated onto the service (pyRevitLabs.PyRevit / PyRevitConfigs): all config access now goes through IConfigurationService; the bespoke CLI parser is removed.

Python config layer migrated (userconfig.py, coreutils/configparser.py, labs.py): the Python layer is now a thin adapter
over the same service and drops the MadMilkman.Ini dependency. The public user_config API is unchanged.

Reconciliation and hardening (deltas applied on top of the #2482 base so the port matches current develop behavior):

  • Re-applied settings added to develop since refactor: Rewrite pyRevit configurations #2482 forked: close-output modes, new_loader, read_script_metadata, cached attachment lookup, and the [Bug]: Install Extensions Path not changeable #3193 third-party-extension ordering.
  • Config discovery prefers the per-user (%APPDATA%) file and falls back to a read-only all-users (%ProgramData%) config, detected via a real write-probe rather than the read-only file attribute alone.
  • Tolerant reads: a single malformed or legacy value now falls back to its default instead of aborting the whole config load.
  • Code-review correctness fixes: a partial section save no longer strips sibling keys; the UTC-timestamp setter writes the correct key; absent-option presence detection is restored.

What this PR intentionally does not include

  • The C# loader still uses its own reader. Unifying the third reader (pyRevitExtensionParser) onto the shared service - and serving one process-wide instance to every engine - is Phase 3.
  • Symmetric value fidelity and the one-time migration are not here yet. This phase carries refactor: Rewrite pyRevit configurations #2482's known string double-encoding behavior. - Fixed in Phase 2
  • refactor: Rewrite pyRevit configurations #2482's JSON and YAML backends are dropped. refactor: Rewrite pyRevit configurations #2482 shipped three pluggable backends - Configurations.Ini, Configurations.Json, and Configurations.Yaml (a ~150-line YamlConfiguration built on YamlDotNet). pyRevit's configuration is INI, so this PR ports only the INI backend and omits the JSON and YAML projects entirely: no source, no project, no format-dispatch left dangling, and no new YamlDotNet/JSON-format dependency added. The pluggable-backend abstraction (IConfiguration) is kept, so a backend could be re-introduced later if there were ever a reason to. Let me know if this is a feature we want to include after phase 3.

Per-Revit-version overrides - ported write-path only

#2482 introduced the ability to override a setting for a specific Revit version (e.g. pyrevit configs rocketmode enable 2025, writing to a versioned file pyRevit_config.2025.ini that takes effect when running in that Revit version). That feature is ported here partially - the write path and the layering plumbing, but not the read path - so it is not yet functional end-to-end:

  • Present (write side): the configs CLI commands accept the optional [<revit_year>] argument; ~35 setters take a revitVersion and save into a per-version config layer; ConfigurationService reads layered sources with the override winning.
  • Missing (read side): every getter is parameterless and reads only the base config (40 no-arg GetConfigFile() reads vs 35 versioned GetConfigFile(revitVersion) writes — no read passes a version). The loader, which is the component that actually runs inside a known Revit version, hard-codes the default layer; Python reads the default; and the CLI has no way to read a version-specific value back. Migration also operates on the base config only.

Net: a versioned pyrevit configs write produces a pyRevit_config.{year}.ini that nothing currently reads. Completing the read-side wiring is deferred to Phase 3 (have the loader resolve its Revit version and request the matching layer; add versioned read overloads), where everything already routes through one shared service keyed by configuration name. It is called out here so reviewers don't mistake the CLI surface for a working feature.

Known limitations

This branch is developed and reviewed in phases but ships as a whole - none of it is released until all phases land. Two defects inherited from #2482 are present at this phase and fixed in later ones; they are called out here so the
gaps are tracked rather than hidden:

  • String double-encoding - two pyRevitLabs.Configurations.Ini round-trip tests are intentionally left red as a canary. Fixed in Phase 2 (symmetric-JSON value contract), which flips them green.
  • Sparse-section save clobber (data loss) — the typed sections still carry field-initializer defaults, so a single-field save (e.g. any pyrevit configs <x> CLI command, which passes a one-field CoreSection) rewrites the whole section and wipes sibling keys such as userextensions. Fixed in Phase 3 by moving section read-defaults to [DefaultValue]/read-time so a sparse save only writes the fields the caller set. Do not run pyrevit configs commands against a config you care about until that phase lands.

Testing

  • C# builds clean: pyRevitLabs.PyRevit and the CLI, both net48 and net8.0.
  • Python py_compile is clean across the migrated modules.
  • New abstraction and INI test suites pass, except the two canary tests noted above.
  • The loader is unchanged in this phase (still on its own reader), so loader behavior is unaffected.

Future work (separate, incremental PRs)

Phase 2 - fidelity & migration

Goal: make the new store byte-faithful to existing config files and self-healing, so adopting it neither loses nor corrupts data - and fix the string-encoding defect inherited from #2482 (turning the two red canary tests green).

Main changes:

  • Symmetric-JSON value contract - a value is JSON-encoded once on write and decoded once on read, consistently on the C# and Python sides. Fixes the double-encoding and flips the two red Ini round-trip tests green.
  • One-time canonicalizing migration, stamped with a [core] config_version key - backs up first, then repairs over-escaped/corrupt values and drops unreadable keys, folding the legacy Python fixup chain into one place.
  • Self-heal on load - a corrupt value is repaired on any writable load, not only at the version-gated migration.
  • Golden-file fidelity corpus - round-trip tests over real-world config shapes (populated, corrupted, legacy, empty) guarding decode/encode parity.
  • Type & contract fixes - store apptelemetry_event_flags as a string (128-bit hex overflows int); ConfigSection.__getattr__ raises AttributeError when an option is absent (restores presence detection).
  • Diagnostics sink - route migration repairs, read-only-admin use, and tolerant-read fallbacks through a logging hook instead of swallowing them.

These fixes address:

Phase 3 — loader integration & one shared instance

Goal: retire the third (loader) reader and serve one process-wide config instance to the loader, CLI, and all engines - eliminating the per-engine re-read/re-save of the config file that originally motivated this work - and fix the sparse-save clobber.

Main changes:

  • One process-wide cached ConfigurationService - discovered and migrated once, shared by the loader, CLI, IronPython, and CPython; "reload" becomes an explicit cache invalidation rather than a re-parse on every engine startup.
  • Config discovery hoisted into the lightweight Configurations.Ini layer so the loader can use the shared service without taking heavy dependencies.
  • Loader reader retired - pyRevitExtensionParser's config becomes a thin adapter over IConfiguration and the standalone Win32 INI reader is deleted.
  • Sparse-save clobber fixed - section read-defaults move to [DefaultValue]/read-time, so a single-field save writes only the fields the caller set (no more wiping userextensions).
  • Cross-reader parity test - loader, CLI, and Python decode one fixture identically, guarding against a fourth divergence.

Future features - if wanted

  • Per-Revit-version configs finished
  • JSON or YAML backend support

@ChrisCrosley
feat(config): add pyRevitLabs.Configurations abstraction and INI backend
7ffc2db 
Port the config abstraction from pyrevitlabs#2482 (dosymep), scoped to INI only;
Json/Yaml backends omitted. Net-new assemblies plus wiring:
- Directory.Build.targets: map net8.0 -> netcore.
- pyRevitLabs.sln: register both projects.
- .gitignore: un-ignore Configurations.Ini/ (matched by *.ini on
  case-insensitive filesystems).
@ChrisCrosley
test(config): add configuration abstraction and INI backend tests
110e54b 
Ports the ConfigurationService and IniConfiguration test projects from
pyrevitlabs#2482. Json/Yaml test projects omitted with their backends.
@ChrisCrosley
refactor(cli): migrate config access to IConfigurationService
7396db6 
Rewire the CLI and pyRevitLabs.PyRevit config consumers onto the new
configuration service; remove the bespoke CLI config reader.

- PyRevitConfigs: rewritten over IConfigurationService / typed sections.
- Remove pyRevitLabs.PyRevit/PyRevitConfig.cs (superseded).
- Port PyRevitAttachments, PyRevitCaches, PyRevitClones, PyRevitExtensions.
- csproj: reference the Configurations assemblies (Json backend dropped),
  keep develop's LibGit2Sharp 0.31.0.

Reconciled against current develop (3-way merge, not a straight port):
- Preserve develop's install-scope ConfigFilePath (IsInstallAllUsers
  marker) over the PR's simplification.
- Re-apply develop's attachment session cache (GetAttachedCached /
  ClearAttachmentCache) and clone bin-artifact install + --skip-bin.
- Add close-output config (GetCloseOutputMode/GetCloseOtherOutputs +
  OutputCloseMode enum + CoreSection keys) consumed by ScriptConsole.
@ChrisCrosley
refactor(runtime): migrate Python config layer to IConfigurationService
e79de95 
Replace the Python-side config reader with a thin wrapper over the shared
C# configuration service.

- userconfig.py: PyRevitConfig now wraps IConfigurationService; typed
  Core/Routes/Telemetry section access; _SectionCompatWrapper preserves
  get_option/set_option for extensions. Module init no longer runs the
  upgrade normalize loop or an unconditional save_changes(): opening Revit
  no longer rewrites the ini.
- configparser.py: ConfigSection/ConfigSections over the service; the
  JSON-over-INI fixup chain moves to the C# IniConfiguration backend.
- labs.py: drop MadMilkman.Ini (package removed); reference
  pyRevitLabs.Configurations / ConfigurationService.
- revit/tabs.py: tolerate non-string/malformed config values.
- loader/sessioninfo.py: drop config_type/config_file log line.

The PR's unrelated runtime DLL-resolution change is intentionally not
ported. Settings reconciliation (new_loader, read_script_metadata,
output close mode) follows in the next commit.
@ChrisCrosley
refactor(config): reconcile settings added to develop since pyrevitla…
a50bd97 
…bs#2482 base

Re-add config surface the PR predated, so existing consumers (sessionmgr,
Settings dialog) keep working:

- CoreSection: new_loader, read_script_metadata typed keys (close-output
  keys were added with the C# migration commit).
- userconfig: new_loader, read_script_metadata, output_close_others,
  output_close_mode_enum properties (typed Core access).
- userconfig.get_thirdparty_ext_root_dirs: restore pyrevitlabs#3193 deterministic
  ordering (default path first) over the PR's set-based version.
- userconfig.get_current_attachment: restore the cached lookup
  (GetAttachedCached) the PR regressed.
- script.py: docstring class rename (ConfigSection).
@ChrisCrosley
fix(config): prefer per-user config, use admin config read-only
d98fa68 
GetConfigFile resolved the user config via the install-scope ConfigFilePath,
which points to ProgramData when the all-users marker is present. A
non-elevated Revit session then tried to write there ("access denied") and
ignored the real per-user APPDATA config.

- Resolve the writable user config from APPDATA (PyRevitPath) directly, so an
  existing per-user config always wins (matches pyRevit's historical Python
  discovery).
- When only an admin (ProgramData) config exists, probe actual writability
  (FileInfo.IsReadOnly misses ACL denials) and open it read-only instead of
  writing; otherwise seed it into the per-user location.
@ChrisCrosley
fix(config): tolerate malformed/legacy values on read
e090b76 
A value that fails JSON deserialization (e.g. escape-doubling corruption
in older configs, such as a backslash-mangled environment.clones dict)
threw out of GetValueOrDefault and aborted the entire config/section load.
Fall back to the default instead, matching pyRevit's historical read
tolerance. GetValue (non-default) still throws.
@ChrisCrosley
fix(config): address code-review correctness bugs
cf01128 
- ConfigurationService.SaveSection: stop RemoveOption on null properties so a
  partial-section save (PyRevitConfigs.Set* single-field records) no longer
  strips the other keys in that section.
- ConfigurationService.GetSectionKeyValueOrDefault: return GetValueOrDefault
  instead of GetValue (was throwing on missing keys despite its name).
- PyRevitConfigs.SetUTCStamps: write TelemetryUseUtcTimeStamps, not
  TelemetryStatus (copy-paste toggled telemetry on/off).
- userconfig.apptelemetry_event_flags: guard None (hex(None) crashed at
  telemetry startup).
- userconfig.reload(): restore the method (get_config(reload=True) raised
  TypeError); re-reads from disk.
- configparser.get_option: tolerate non-JSON/legacy values instead of letting
  json.loads crash config reads.
@ChrisCrosley
docs(config): reword comments to describe behavior, not change rationale
5fdaa9c 
Comments authored during the port/fixes were describing why a change was
made or referencing prior code. Reword them to state what the current code
does (SaveSection null handling, GetConfigFile discovery, IsFileWritable,
GetValueOrDefault fallback, ext-dir ordering, get_option non-JSON handling).
@devloai

devloai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Unable to trigger custom agent "Code Reviewer". You have run out of credits 😔
Please upgrade your plan or buy additional credits from the subscription page.

@jmcouffin
jmcouffin requested a review from Copilot June 23, 2026 20:04
Copilot AI reviewed Jun 23, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new shared configuration abstraction (pyRevitLabs.Configurations + pyRevitLabs.Configurations.Ini) and migrates the CLI + Python config layer to use it, with the goal of unifying INI parsing/defaults and reducing duplicated readers across components.

Changes:

  • Added pyRevitLabs.Configurations abstractions + typed section POCOs, and an INI backend implemented on ini-parser-netstandard.
  • Migrated CLI (pyRevitLabs.PyRevit, pyRevitCLI) and Python config adapter (pyrevitlib/pyrevit/userconfig.py, coreutils/configparser.py) off the bespoke INI readers.
  • Added new xUnit test projects for the abstraction and the INI backend, and updated solution/build wiring.

Reviewed changes

Copilot reviewed 43 out of 45 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
pyrevitlib/pyrevit/userconfig.py Refactors Python user_config to wrap the C# configuration service and typed sections.
pyrevitlib/pyrevit/script.py Updates get_config() docstring type to the new ConfigSection wrapper.
pyrevitlib/pyrevit/revit/tabs.py Hardens tab-coloring config reads against malformed types/values.
pyrevitlib/pyrevit/labs.py Removes MadMilkman.Ini dependency and references the new Configurations assembly.
pyrevitlib/pyrevit/coreutils/configparser.py Replaces Python-side INI parsing with a thin adapter over the C# configuration service.
dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Tests/pyRevitLabs.Configurations.Tests.csproj Adds new xUnit project for configuration abstraction tests.
dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Tests/ConfigurationTests.cs Adds baseline tests for IConfiguration behaviors.
dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Tests/ConfigurationServiceUnitTests.cs Adds (currently empty) unit test harness for ConfigurationService.
dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Tests/ConfigurationServiceFixture.cs Adds fixture for constructing ConfigurationService in tests.
dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Ini.Tests/pyRevitLabs.Configurations.Ini.Tests.csproj Adds xUnit project for INI backend tests.
dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Ini.Tests/IniCreateFixture.cs Adds test fixture that creates/deletes a temp INI file.
dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Ini.Tests/IniConfigurationUnitTests.cs Adds unit tests for INI configuration creation/builder validation.
dev/pyRevitLabs/pyRevitLabs.sln Registers new projects and adds Any CPU/x86 configs and solution folders.
dev/pyRevitLabs/pyRevitLabs.PyRevit/pyRevitLabs.PyRevit.csproj Removes MadMilkman.Ini packages and references the new Configurations projects; deploys INIFileParser if present.
dev/pyRevitLabs/pyRevitLabs.PyRevit/PyRevitExtensions.cs Migrates extension enable/disable and extension path handling to IConfigurationService and section POCO saves.
dev/pyRevitLabs/pyRevitLabs.PyRevit/PyRevitConsts.cs Minor whitespace/comment formatting changes only.
dev/pyRevitLabs/pyRevitLabs.PyRevit/PyRevitConfigs.cs Replaces the old config reader with ConfigurationBuilder + INI configuration sources; adds read-only detection via write-probe; threads optional “revitYear” layer through setters.
dev/pyRevitLabs/pyRevitLabs.PyRevit/PyRevitConfig.cs Deletes the old MadMilkman-backed config reader.
dev/pyRevitLabs/pyRevitLabs.PyRevit/PyRevitClones.cs Migrates clone registry persistence to typed EnvironmentSection.
dev/pyRevitLabs/pyRevitLabs.Configurations/Sections/TelemetrySection.cs Adds typed telemetry section definition.
dev/pyRevitLabs/pyRevitLabs.Configurations/Sections/RoutesSection.cs Adds typed routes section definition.
dev/pyRevitLabs/pyRevitLabs.Configurations/Sections/EnvironmentSection.cs Adds typed environment section definition.
dev/pyRevitLabs/pyRevitLabs.Configurations/Sections/CoreSection.cs Adds typed core section definition and defaults.
dev/pyRevitLabs/pyRevitLabs.Configurations/pyRevitLabs.Configurations.csproj Adds multi-targeted Configurations project (net48/net8.0) and InternalsVisibleTo for tests.
dev/pyRevitLabs/pyRevitLabs.Configurations/Extensions/ConfigurationExtensions.cs Introduces placeholder extension class (currently empty).
dev/pyRevitLabs/pyRevitLabs.Configurations/Exceptions/ConfigurationSectionNotFoundException.cs Adds custom exception for missing sections.
dev/pyRevitLabs/pyRevitLabs.Configurations/Exceptions/ConfigurationSectionKeyNotFoundException.cs Adds custom exception for missing keys.
dev/pyRevitLabs/pyRevitLabs.Configurations/Exceptions/ConfigurationException.cs Adds base configuration exception type.
dev/pyRevitLabs/pyRevitLabs.Configurations/Constants.cs Adds internal constants for env-related section/key names.
dev/pyRevitLabs/pyRevitLabs.Configurations/ConfigurationService.cs Adds the configuration service for layering + section (de)serialization via attributes.
dev/pyRevitLabs/pyRevitLabs.Configurations/ConfigurationName.cs Adds internal record to track layered config names/order.
dev/pyRevitLabs/pyRevitLabs.Configurations/ConfigurationBuilder.cs Adds builder for composing layered IConfiguration sources into a service.
dev/pyRevitLabs/pyRevitLabs.Configurations/ConfigurationBase.cs Adds base class implementing common IConfiguration behaviors + tolerant reads.
dev/pyRevitLabs/pyRevitLabs.Configurations/Attributes/SectionNameAttribute.cs Adds attribute for binding POCOs to INI section names.
dev/pyRevitLabs/pyRevitLabs.Configurations/Attributes/KeyNameAttribute.cs Adds attribute for binding POCO properties to INI key names.
dev/pyRevitLabs/pyRevitLabs.Configurations/Abstractions/IConfigurationService.cs Adds service contract for layered configurations + typed sections.
dev/pyRevitLabs/pyRevitLabs.Configurations/Abstractions/IConfiguration.cs Adds configuration backend contract (read/write/serialize).
dev/pyRevitLabs/pyRevitLabs.Configurations.Ini/pyRevitLabs.Configurations.Ini.csproj Adds INI backend project targeting net48/net8.0 with ini-parser and pyRevitLabs.Json reference.
dev/pyRevitLabs/pyRevitLabs.Configurations.Ini/IniConfiguration.cs Implements IConfiguration using ini-parser with JSON-based value serialization and some legacy parsing logic.
dev/pyRevitLabs/pyRevitLabs.Configurations.Ini/Extensions/IniConfigurationExtensions.cs Adds builder extension for registering INI configurations.
dev/pyRevitLabs/pyRevitCLI/Resources/UsagePatterns.txt Extends pyrevit configs usage patterns to accept optional <revit_year>.
dev/pyRevitLabs/pyRevitCLI/PyRevitCLIExtensionCmds.cs Threads revit version layer into extension enable/disable calls.
dev/pyRevitLabs/pyRevitCLI/PyRevitCLI.cs Threads <revit_year> through config setters and extension toggle path.
dev/Directory.Build.targets Adds NetFolder mapping for net8.0 to netcore output folder.
.gitignore Un-ignores the new pyRevitLabs.Configurations.Ini directory from the existing *.ini ignore rule.

Comment thread pyrevitlib/pyrevit/userconfig.py Outdated
Comment thread pyrevitlib/pyrevit/userconfig.py Outdated
Comment thread pyrevitlib/pyrevit/userconfig.py Outdated
Comment thread pyrevitlib/pyrevit/coreutils/configparser.py
Comment thread dev/pyRevitLabs/pyRevitLabs.Configurations/Abstractions/IConfiguration.cs
Comment thread dev/pyRevitLabs/pyRevitLabs.Configurations/Sections/CoreSection.cs
Comment thread dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Tests/ConfigurationTests.cs
Comment thread dev/pyRevitLabs/pyRevitLabs.Configurations.Ini/Extensions/IniConfigurationExtensions.cs
Comment thread pyrevitlib/pyrevit/revit/tabs.py Outdated
Comment thread pyrevitlib/pyrevit/revit/tabs.py Outdated
@ChrisCrosley
fix(config): address Copilot PR review comments
da649b2 
- userconfig: config_file reports the service-resolved path, not the
  fixed install-scope path
- userconfig: _SectionCompatWrapper.get_option tolerates non-JSON values
  instead of raising
- userconfig: fix remove_section body indentation
- versionmgr/upgrade: remove dead upgrade_user_config + telemetry-heal
  helpers (and the now-unused constants/imports)
- tabs: resolve tab-style index through a tolerant fallback to the
  default index on malformed/out-of-range config
- Configurations: make the IConfiguration Type-based overloads public
- Configurations.Ini: parse hex integers as Int64 so long targets do
  not overflow
- Configurations: pass (keyName, sectionName) to
  ConfigurationSectionKeyNotFoundException so its fields are correct
- Configurations.Ini: fix conigurationName parameter typo
- tests: drop duplicate apptelemetry_event_flags SetValue in fixture
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ChrisCrosley