refactor: new pyRevit C# config service by ChrisCrosley · Pull Request #3450 · pyrevitlabs/pyRevit

ChrisCrosley · 2026-06-23T20:00:20Z

Edit: The original description outlined 3 phases. All work is complete and I've rewritten this description.

Summary

pyRevit_config.ini is read by three independent parsers: the Python layer (pyrevit.coreutils.configparser), the CLI (pyRevitLabs.PyRevit), and the C# loader (pyRevitExtensionParser). Each has its own defaults and quirks, and they have drifted apart. None share a parsed result, so the file is re-read for every startup script and smartbutton engine — at least five reads on a basic install, 200–500ms each.

This branch replaces all three with one C#-owned configuration store shared by the loader, CLI, IronPython, and CPython. Legacy INI parsing moves into a version-stamped migrator that repairs a config in place.

Port of @dosymep's #2482 onto current develop, reconciled with what has changed since, plus install-scope awareness, migration, and loader integration.

Area	Files	Added	Removed	Net
C# (excluding tests)	36	+3,581	−1,736	+1,845
Python (excluding tests)	11	+684	−856	−172
Tests (C#, Python, fixtures, test buttons)	27	+2,936	−246	+2,690
Build, project, CI, CLI usage	9	+258	−6	+252
Total	83	+7,459	−2,844	+4,615

The service

New pyRevitLabs.Configurations assembly: IConfigurationService / IConfiguration with attribute-bound typed sections ([SectionName] / [KeyName] / [DefaultValue]) for [core], [routes], [telemetry], [environment], and per-extension sections. Defaults live on the section schema instead of being restated by each reader.

A value that fails to decode falls back to its default instead of aborting the whole section load (GetValueOrDefault).
Typed section snapshots rebuild when a write advances the store's revision (EnsureSnapshots), so a reader never sees state older than the last write.
Read-only configs reject writes up front (EnsureWritable) rather than dropping them at flush time.

The INI backend

pyRevitLabs.Configurations.Ini reads and writes UTF-8 without a BOM so Python's configparser can read the same file.

A value is JSON-encoded once on write and decoded once on read, on both the C# and Python sides. That closes the escape-doubling growth in #3334.
Legacy encodings still read without a rewrite: Python True/False, hex integers, bare unquoted strings and Windows paths, single-quoted list literals. The list form (LegacyListFormat) and the clone-registry dict form (LegacyDictFormat) are each defined once and shared by the read path and the migrator, so both interpret them identically.

Migration

ConfigurationMigrator runs on any writable load, stamped with [core] config_version, so corruption introduced after first run still gets repaired. Several recent issues trace back to config parsing; this gives those fixes one home.

Scans before mutating — a clean, already-stamped config performs no write.
Drops typed values that no longer parse to their declared type, and telemetry fields left as escape-doubling wreckage. Detection is by shape, not just length: a value made up entirely of quote, escape, and slash artifacts is unrecoverable at any size, which is what an emptied field degrades into at 14–16 characters, well under the 8192-char threshold (#3534, heuristic from #3536). Short legitimate paths and URLs, the canonical "", and legacy bare values are left alone.
Canonicalizes legacy lists and the clone dict to JSON, idempotently. A literal containing a double quote is skipped, so a path with an apostrophe isn't rewritten and re-backed-up on every load.
Backs up before mutating; if the backup can't be written the run is skipped and retried on a later load (Migrate).

Consumers

CLI (PyRevitConfigs.cs) — all access goes through IConfigurationService; the bespoke CLI parser (PyRevitConfig.cs, −176 lines) is deleted.

Loader (PyRevitConfig.cs) — now a read-only facade over IConfiguration; the standalone Win32 IniFile.cs reader is deleted (−460 lines). Values decode through the service, so the loader agrees with the CLI and Python. Its cached instance is held only while it still wraps the configuration the shared store hands out, so a reload from any host is observed (Load).

Python (userconfig.py, coreutils/configparser.py) — thin adapters over the same service; the MadMilkman.Ini dependency is gone. Typed section properties write through on assignment, and save_changes flushes once. versionmgr/upgrade.py drops to the 4.8.5 temp-file cleanup. 34 of userconfig.py's 40 accessors are now one-line delegations and could be deprecated once extensions move to the typed section.name form.

Behavior changes

apptelemetry_event_flags is stored as a string, not an int — the 128-bit hex bitmask overflows int (TelemetrySection). Legacy 0x… values still read.
Files written by this branch carry [core] config_version and, on first repair, leave a .v0.<timestamp>.bak alongside.
On an all-users install, %ProgramData% is the writable target only for an elevated process. Standard users resolve to a per-user config under %APPDATA%, seeded from the machine config — the policy develop settled in #3512 and #3523. A machine config a process can't write is not treated as a lockdown; only the DOS ReadOnly attribute is (pyrevit configs seed --lock).
Split-config repair follows from that: it runs only when elevated, since a standard user's %APPDATA% config is their own active config. When it moves the clone registry or a missing extension section into the machine config, the per-user file is renamed to pyRevit_config.ini.split-admin.<timestamp>.bak; one with nothing to contribute is left in place. Only the clone registry and extension sections are merged, so that user's [core], [routes], and [telemetry] values stay in the retired copy.

Breaking changes

The surface extensions actually use is unchanged: user_config, CONSTS, the snake_case accessors, get_section/add_section/has_section/remove_section, save_changes, reload, and get_option/set_option/has_option/remove_option. Nothing in this repo uses a removed name, and none of the 25 extensions in extensions.json do either — only two touch the config service at all.

Internals removed:

coreutils.configparser — PyRevitConfigParser / PyRevitConfigSectionParser, replaced by ConfigSections / ConfigSection. For a tool keeping settings in its own ini, open_config_file(path) returns ConfigSections over any ini path.
userconfig module level — the always-empty path placeholders (CONFIG_FILE, USER_CONFIG_FILE, ADMIN_CONFIG_FILE, LOCAL_CONFIG_FILE; use user_config.config_file), and find_config_file / verify_configs.
PyRevitConfig — get_config_version() (unrelated to the new migration stamp) and get_config_file_hash(). Its constructor now takes the service rather than a file path.
versionmgr.upgrade — upgrade_user_config / heal_bloated_telemetry_fields, now the migrator's job; upgrade_existing_pyrevit() is unchanged. Two deliberate differences: the decode/re-encode sweep over every option is not ported, since re-encoding an already-mangled value is the escape-doubling mechanism itself — the migrator rewrites only keys it has positively identified. And a healed telemetry field is now removed so it falls back to its section default, where the Python version wrote ''; identical except for apptelemetry_server_url, which declares no default and reads back null.

Testing

The .NET suites run in CI now (Run configuration unit tests, Run config parity tests).

pyRevitLabs.Configurations.Tests (xUnit, 14) — extension-section resolution; PyRevitConfigStore caching, concurrency, failed-build eviction, reset.
pyRevitLabs.Configurations.Ini.Tests (xUnit, 104) — golden-file round-trips over four real config shapes, list decoding across every historical encoding, malformed-file tolerance, read-only guards, migration of both legacy lists and the clone dict, telemetry repair, snapshot freshness, discovery. Selection is covered across install scope and elevation, including that a machine config a standard user can't write resolves to a writable per-user copy (#3504). Split-admin repair is driven end to end, including an inert second pass.
ConfigParityTests / PyRevitConfigsFacadeTests (NUnit, 17, run by filter) — CLI get-after-set verified against a fresh read of the file, so a write is proven to reach disk; loader surface checked against the service over a fixture where every value contradicts its section default.
test_config_roundtrip.py (66) — runs under IPY2, IPY3, and CPython. 56 are hermetic against a dict-backed fake IConfiguration, including container values whose Windows paths carry unescaped backslashes (JSON rejects them, so both readers retry escaped) asserted down to the original failure: a path list decoding to a string and iterating one character at a time. The other 10 need the real INI backend and skip when the labs assemblies aren't loadable — 7 re-run the key assertions so the fake can't drift, 3 cover open_config_file.
"Config Module Tests" DevTools button — runs every test_config_* module inside a live Revit session.

Out of scope

#2482's JSON and YAML backends are dropped. pyRevit's config is INI, so only that backend is ported — no YamlDotNet dependency, no dangling format dispatch. IConfiguration stays, so another backend can be added through it later.
Per-Revit-version overrides are dropped. #2482 added versioned override files but implemented only the write half, so the command reported success and did nothing. Full support belongs in its own PR.
The dormant test suites stay dark. This predates the branch. The only test command in any workflow is dotnet test tests/Build.Tests.csproj, which covers the build pipeline, not product code. pyRevitExtensionParserTester (~297 NUnit tests) builds with its results discarded — this PR runs 17 of them — and pyRevitLabs.UnitTests (31 MSTest) is solution-referenced only and touches install-scope and addon paths, so it needs an audit for which cases are hermetic.

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).

Ports the ConfigurationService and IniConfiguration test projects from pyrevitlabs#2482. Json/Yaml test projects omitted with their backends.

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.

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.

…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).

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.

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.

- 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.

Golden-file corpus + read/round-trip assertions over real-world config shapes (populated, corrupted clones, legacy formats, empty). Acceptance_* tests encode the Phase 2.1 symmetric-JSON target and are RED until it lands (C# string reads must be decoded, not JSON-quoted); the rest assert current Phase 1 behavior and pass. - Deploy pyRevitLabs.Json (Private=false in the Ini backend) to the test output so the suite runs standalone. - .gitignore: un-ignore the test .ini fixtures (matched by *.ini).

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).

Strings are JSON-encoded once and decoded on read on both sides, so C# typed string access and the typed POCO sections no longer read back quoted. - IniConfiguration.GetValueImpl: deserialize strings (drop the raw-return special case). - IConfiguration/ConfigurationBase: add GetRawValueOrDefault/SetRawValue raw accessors; IniConfiguration stores/returns the value text unchanged. - configparser.ConfigSection and userconfig._SectionCompatWrapper: read/write via the raw accessors (json.loads / json.dumps once), removing the double-encode that fed escape-doubling. Golden-file string-contract tests now pass (18/18).

Repairs an existing config on first writable load: drops typed-section values that no longer parse to their declared type (e.g. an escape-doubled environment.clones dict), resets telemetry fields blown up by escape-doubling, then stamps [core] config_version so it runs once. - ConfigurationMigrator (version-gated, backs up before mutating). - PyRevitConfigs.GetConfigFile runs it on the writable user config only; admin/read-only configs are left untouched. - Remove the orphaned Python upgrade_user_config / heal_bloated_telemetry_fields; that work now lives in the migrator. - Golden-file tests: corrupt-value repair + idempotency.

…eError - TelemetrySection.AppTelemetryEventFlags int -> string: the field is a 128-bit hex string and overflowed Int32. PyRevitConfigs and userconfig pass it through as text (no hex parse/format). - ConfigSection.__getattr__ raises AttributeError for an absent option (was returning None), restoring hasattr / presence detection. - Golden-file fixture/test: large hex flags round-trip as a string.

- ConfigurationMigrator.Migrate returns a result (reset keys, backup path, backup-failed flag) and aborts when an existing file cannot be backed up, so it never mutates without a recoverable copy. - PyRevitConfigs logs the migration (Info) and each reset key (Warn), warns when migration is skipped for lack of a backup, and reports use of a read-only admin config (Info) since user changes are not saved.

GetValueOrDefault silently returned the default when a stored value failed to deserialize. Add a static ConfigurationDiagnostics sink (the assembly has no logger of its own); PyRevitConfigs routes it to logger.Warn, so a silently-defaulted value now leaves a trace even on read-only configs the migration cannot repair.

The migrator scanned and stamped a version once; a value corrupted after that stamp warned on every read but was never removed. Now the repair runs whenever an unreadable value is present on a writable config, independent of the version stamp. A config with nothing to fix performs no write, so a clean load still does not rewrite the ini.

Every config getter/setter rebuilt the service and re-ran migration on each call, and each engine re-read and re-saved the file. Introduce a process-wide cache so the loader, CLI, and script engines read one in-process instance, and hoist config discovery into the lightweight Configurations.Ini layer so the loader can share it without depending on the heavier pyRevitLabs.PyRevit or pyRevitLabs.Common assemblies. - Add PyRevitConfigStore: caches the built service by configuration name (default-name variants collapse to one instance); Reload invalidates. - Add PyRevitConfigService and PyRevitConfigPaths in Configurations.Ini: the default build factory (discovery, all-users fallback, seeding, migration) and file-location helpers, with no dependency on pyRevitLabs.Common. - PyRevitConfigs delegates GetConfigFile/ReloadConfig to the shared service and routes Configurations diagnostics to the pyRevit log. - ConfigurationService.SaveSection refreshes the typed section snapshots after a save so readers of the shared instance observe the write; userconfig save_changes captures the section snapshots before the sequential save so the refresh cannot drop pending edits. Tests: shared-store caching/reload, config-path discovery, and snapshot refresh on a real INI-backed service.

Point the loader's PyRevitConfig at the process-wide PyRevitConfigService instead of its own INI reader, so the loader, CLI, and Python engines share one in-process instance and one discovery path. PyRevitConfig becomes a thin adapter over IConfiguration with the same public surface and parsing semantics, using JSON-or-raw tolerant decoding so both migrated (JSON-encoded) and legacy (bare) values read correctly. ParseExtensionByName reads per-extension sections through the configuration; custom-path Load() stays non-shared for tests; ClearCache also drops the shared service cache. The old IniFile reader is left in place (now unused) next to the still-used PythonListParser, pending the in-Revit smoke test.

The loader now depends on pyRevitLabs.Configurations and .Configurations.Ini. Exclude them and their INIFileParser dependency from the per-engine-folder deploy so they load once from the bin/{netcore,netfx} root via LoadAssembliesInFolder, matching how pyRevitLabs.Common is handled and avoiding a duplicate/skewed load from two paths.

A per-key setter builds a sparse section POCO (e.g. new CoreSection { RocketMode = x }), but SaveSection wrote every non-null property -- so the section's field- initializer defaults (rocketmode, userextensions, port, sources, clones, ...) overwrote or wiped sibling keys the caller never touched. Move section read-defaults off field initializers: declare scalar defaults via [DefaultValue] and supply an empty instance for collection properties, applied by CreateSection on read. Unset properties are now null, so SaveSection's existing null-skip writes only the keys a caller set -- and setting a property to its own default value still persists, since it is explicitly non-null. Read-time default values are unchanged.

…ervice Add ConfigParityTests: the loader's PyRevitConfig adapter and the shared ConfigurationService must decode the same canonical config file into identical values. The CLI and Python engines read through the same ConfigurationService, so loader/service parity transitively covers all readers and guards against a fourth reader ever drifting. Runs on net48 and net8.0-windows.

PyRevitConfig now reads through the shared configuration service, so the Win32 INI reader is unused. Remove it and move the still-used PythonListParser into its own file.

devloai · 2026-06-23T20:00:24Z

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.

Copilot

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.

- 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

- Config-file discovery regex: `.*[pyrevit|config].*\.ini` used a character clas,. Use `(pyrevit|config)` instead. Fixed in both resolvers (PyRevitConsts and PyRevitConfigPaths). - extension.json bool parsing: BuiltIn/DefaultEnabled/ RocketModeCompatible called bool.Parse directly, throwing on a missing field or non-"true"/"false" string. Parse defensively with per-field defaults (builtin=false, default_enabled=true, rocket_mode_compatible=false)

Reconcile the config-store rewrite with develop's install-scope/config-split (pyrevitlabs#3452) and loader-auth (pyrevitlabs#3461) work. Kept the branch's config-store architecture; ported seedshippeddefaults to the new IConfigurationService API and re-added parser PyRevitConfig.ExtensionLookupSources for the auth path. Dropped the pyrevitlabs#3441 admin split-config migration (depends on the deleted PyRevitConfig class) as a tracked follow-up. Builds on net48+net8.0; config and parity test suites pass.

romangolev · 2026-08-20T16:45:15Z

@ChrisCrosley got a couple of things updated and resolved the merge conflict

jmcouffin · 2026-08-20T18:11:45Z

Did someone tested it using pyrevit and it's UI and config?
I haven't checked the code yet. I'm just wondering if everything is properly wired.
@ChrisCrosley @Wurschdhaud @romangolev

romangolev · 2026-08-20T18:17:28Z

@jmcouffin I did some smoke tests in Revit 2021-2025 and it worked

Copilot

Pull request overview

Copilot reviewed 84 out of 85 changed files in this pull request and generated no new comments.

sanzoghenzo · 2026-08-21T07:33:25Z

@jmcouffin @romangolev @dosymep

I saw that many edits in this PR are just formatting changes.
Should we decide on the format to use?
Then we could:

add the proper checks, like .editorconfig and/or pre-commit hooks
format everything in a separate PR
add the commit of that PR to the .git-blame-ignore-revs file for a cleaner git blame

romangolev · 2026-08-21T09:19:14Z

I saw that many edits in this PR are just formatting changes.

That was mostly my initiative, we have here mostly formatting regarding to docstrings and in-line comments

add the proper checks, like .editorconfig and/or pre-commit hooks

Anticipating your wishes, I had created a new draft PR addressing this topic

build: enforce ruff format and dotnet format via pre-commit hooks #3581

sanzoghenzo

I'm halfway the review (in terms of number of files viewed, and I started from the smaller ones) and it's already taking too long 😅

As I stated in my previous comment, the many changes in formatting makes it hard to focus on the actual changed code.

sanzoghenzo · 2026-08-21T03:50:23Z

+      - name: Run configuration unit tests
+        run: |
+          dotnet test dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Tests/pyRevitLabs.Configurations.Tests.csproj -c Release
+          dotnet test dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Ini.Tests/pyRevitLabs.Configurations.Ini.Tests.csproj -c Release
+
+      - name: Run config parity tests
+        run: >
+          dotnet test dev/pyRevitLoader/pyRevitExtensionParserTester/pyRevitExtensionParserTest.csproj
+          -c Release --no-build -f net8.0-windows
+          --filter "FullyQualifiedName~ConfigParityTests|FullyQualifiedName~PyRevitConfigsFacadeTests"


Instead of adding things in the ci, It would be better to define the job in the build project via modular pipelines, so that it can also be run locally

sanzoghenzo · 2026-08-21T03:58:40Z

+        /// A config option that is not set reaches this dictionary as a null, so
+        /// every field read below goes through a type-matched <c>GetXxx</c> helper
+        /// rather than an unboxing cast, which would throw and take down session
+        /// load over a single absent option.


What? As I said multiple times before, the documentation should declare what a method does, not how.
And should not refer previous behaviour.
Also, keep it in simple terms.

sanzoghenzo · 2026-08-21T04:45:52Z

+        configuration.SkipInvalidLines = true;
+        configuration.AllowDuplicateSections = true;
+        configuration.AllowDuplicateKeys = true;
+        configuration.OverrideDuplicateKeys = true;


Are we sure we want this?
I get that this is for the migration to follow through, but a malformed INI file shouldn't be allowed to prevent user confusion.
For example, the duplicate keys override could be an oversight and the user might expect the first value to win; if we don't signal the issue in any way, the user is clueless on why things don't work as he/she intended.

sanzoghenzo · 2026-08-21T08:07:40Z

+    /// Parsing never fails on a malformed file: unparseable lines are dropped
+    /// and the last value of a repeated key wins. A config that refused to load
+    /// would take down the loader, CLI, and script engines at once, and would
+    /// never reach the migrator that repairs it.


We're not writing novels here, but technical documentation.
Rephrase to highlight what it does, not the advantages of the implementation choice

sanzoghenzo · 2026-08-21T08:11:58Z

+    protected override void SaveConfigurationImpl(string configurationPath)
+    {
+        string? directory = Path.GetDirectoryName(configurationPath);
+        if (!string.IsNullOrEmpty(directory))


Should we worry (= throw error) about the directory being null or empty?

sanzoghenzo · 2026-08-21T08:52:02Z

+    /// Parses a JSON list literal, retrying with backslash-escaping when the
+    /// first attempt fails, since a legacy Windows path stored unescaped carries
+    /// backslashes JSON treats as bad escapes. Returns false for a value with no
+    /// double quote at all, since that is the Python single-quoted form rather


I surely am biased against AI generated docs, but I feel that the ", since ..." form should not be accepted in code documentation.

sanzoghenzo · 2026-08-21T09:17:21Z

+The flat snake_case accessors (e.g. `user_config.rocket_mode`) are convenience
+aliases over these sections and are considered legacy: prefer the typed
+`section.name` form in new code. The aliases are expected to be deprecated in a
+future release once callers have migrated, so avoid adding new ones.


Has this been already decided?
being a python guy, I would prefer snake_case attributes 😅

This is definitely open to discussion! Just looking at it from the perspective that much of the file can be deleted if we can live without them.

jmcouffin · 2026-08-21T11:14:50Z

@sanzoghenzo 2 commits are marked as the formating ones, I'm sure you could review and using some git mojo to not have that in the way:

sanzoghenzo · 2026-08-21T11:28:27Z

I'm sure you could review and using some git mojo to not have that in the way

Not possible on GitHub web and mobile app, and I don't know how to do reviews locally 😅 I'll try to learn it as soon as I have time

jmcouffin · 2026-08-21T12:02:03Z

@romangolev could you just revert these two commits for convenience for now?

Co-authored-by: Andrea Ghensi <andrea.ghensi@gmail.com>

…n.cs Co-authored-by: Andrea Ghensi <andrea.ghensi@gmail.com>

ChrisCrosley · 2026-08-21T19:55:57Z

@jmcouffin, I know you made several fixes to config parsing/writing in the past couple releases. I believe I ported them over successfully, but probably worth a second opinion.

jmcouffin · 2026-08-21T20:11:42Z

@jmcouffin, I know you made several fixes to config parsing/writing in the past couple releases. I believe I ported them over successfully, but probably worth a second opinion.

Honestly, the quantity of code and refactor is overwhelming. This would probably take me a week to review properly and test. I'll need a a Claude max licence and automated UI testing to validate qprity and code.
This is also why I'm slow to review/merge these days. The quantity exceeds human capacity...
Give me'some time.

ChrisCrosley · 2026-08-21T20:30:06Z

Oh, no rush at all. I just wanted to call that item out specifically because it's the sort of thing that might fall through the cracks.

romangolev · 2026-08-23T17:42:38Z

@jmcouffin this one is really big PR but it mostly follows the effort of

refactor: Rewrite pyRevit configurations #2482

We may ask a help of @dosymep as well to get an opinion of the initial contributor to this issue

dosymep · 2026-08-24T10:18:56Z

@romangolev
If it works, I'm all for it :)

dosymep · 2026-08-24T10:20:04Z

I was removed as the author of these commits? hehe

romangolev · 2026-08-24T13:37:18Z

@dosymep I believe it was easier to start off a new branch. The other one was quite outdated, no hard feelings!

dosymep · 2026-08-24T15:05:30Z

You should have pulled my branch and pushed it, then my authorship would have remained, but it's so offensive

romangolev · 2026-08-24T21:04:12Z

Did someone tested it using pyrevit and it's UI and config?

Tested with Revit 2021 - 2027 @jmcouffin @ChrisCrosley

Tier 1 — Automated suites

Suite	Command	Result
Config core (14 xUnit)	`dotnet test dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Tests/pyRevitLabs.Configurations.Tests.csproj`	✅ 14/14
INI backend (104 xUnit)	`dotnet test dev/pyRevitLabs/tests/pyRevitLabs.Configurations.Ini.Tests/pyRevitLabs.Configurations.Ini.Tests.csproj`	✅ 104/104
Parity/facade (17 NUnit)	`dotnet test dev/pyRevitLoader/pyRevitExtensionParserTester/pyRevitExtensionParserTest.csproj -f net8.0-windows --filter "FullyQualifiedName~ConfigParityTests\|FullyQualifiedName~PyRevitConfigsFacadeTests"`	✅ 17/17
Python round-trip	Ran via pyRevitDevTools "Config Module Tests" button, live Revit session (IronPython 2712 engine)	✅ 60/60

Ran for real via the DevTools button: 60/60 passed, ~0.9s, no
failures. Note: the PR body describes 66 tests (56 hermetic + 10 needing
the real INI backend, "under IPY2, IPY3, and CPython"); this run was a
single pass under the IronPython 2712 engine only, so the count
difference is likely per-engine test selection/skips, not a shortfall —
not confirmed against IPY2/CPython separately. One thing worth noting
from the log: SectionCompatWrapperReadOnlyTests ran with the config in
"admin mode, skipping write" — a side effect of the earlier configs seed --lock testing having left the active config read-only — and those
tests passed correctly under that condition.

Tier 2 — Manual smoke tests

pyrevit below is ./bin/pyrevit.exe from the repo root — it's already
built from this branch (see earlier chat), no rebuild needed.

✅ Fresh install (no config file) starts cleanly with sane defaults.
✅ Existing config migrates: .v0.<timestamp>.bak created, config_version stamped, settings preserved. Idempotent on repeat launch — confirmed once tested without cross-branch contamination (see note above); the two .bak files originally found were from testing feat/forms-dark-mode in between, not a real bug.
✅ Settings UI round-trip ("Save Settings and Reload") — values stick, match disk.
✅ CLI ↔ GUI ↔ loader agreement — write via one surface, read via another.
```
pyrevit configs rocketmode enable
pyrevit configs rocketmode
```
Open pyRevit Settings in Revit → confirm Rocket Mode shows enabled. Flip it off in the Settings UI, "Save Settings and Reload," then:
```
pyrevit configs rocketmode
```
→ should now report disabled.
✅ Extension loading unaffected — ribbon builds identically to develop, no startup errors.
```
pyrevit caches clear --all
pyrevit env
```
Clear caches first so the ribbon is rebuilt from source rather than served from a stale cache — that isolates whether this branch builds it correctly, not whether the cache was already fine. pyrevit env lists installed extensions and attached Revit versions — confirm pyArchitect (or whatever you have installed) shows up with no error markers. Then launch Revit and visually compare the ribbon against what develop shows: same tabs/panels/buttons, no missing icons, no error/warning bubble in the pyRevit panel. There's no CLI-only way to confirm "no startup errors" — that's the one part of this item that has to be eyeballed live.

Tier 3 — Edge cases from the PR body

✅ (short-value half) Escape-doubled telemetry field repair not tested yet, but the short-legitimate-value round-trip is confirmed:
```
pyrevit configs telemetry server https://example.com/telemetry
pyrevit configs telemetry server
```
→ Telemetry Server Url: https://example.com/telemetry. The earlier
"0x0" result was a false positive from testing against the old
globally-installed CLI (v6.1.0) instead of this branch's ./bin/pyrevit.exe.
Corruption-repair half of this item (doubled-escape field) still untested.
✅ Large apptelemetry_event_flags (128-bit hex) round-trips as a string, no int overflow.
```
pyrevit configs apptelemetry flags 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
pyrevit configs apptelemetry flags
```
Confirmed: App Telemetry Flags: 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF — round-trips exactly, no overflow.
✅ (partial) Legacy formats read without rewrite: Python True/False, hex ints, bare strings/paths, single-quoted lists/dicts.
```
pyrevit configs loadbeta
pyrevit configs startuptimeout
```
→ Load Beta is Enabled, Startup log timeout is set to: 15 — both
read back correctly with no crash. The earlier loadbetatools typo
and the startuptimeout null-reference crash were both false
positives from testing against the old CLI (v6.1.0), not this branch.
Still untested: userextensions = ['C:\a','C:\b'] (legacy
single-quoted list) and confirming none of these trip a rewrite/new
.bak on disk.
✅ (partial) All-users install, standard user: writable per-user config under %APPDATA%, seeded from machine config.
From an elevated shell:
```
pyrevit attach shell-smoke default --attached --allusers
pyrevit configs seed
```
Then, from a normal (non-elevated) shell/Revit launch:
```
pyrevit env
```
→ Active Config: "C:\Users\Equipo\AppData\Roaming\pyRevit\pyRevit_config.ini"
— confirms the standard-user session correctly resolves to the
per-user %APPDATA% config rather than %ProgramData%. Not yet
separately confirmed: that this file's values actually came from
seeding the machine config, and that %ProgramData%\pyRevit\pyRevit_config.ini
itself was left untouched by this session.
☐ All-users install, elevated: split-admin repair merges clone/extension sections into machine config, retires per-user file to .split-admin.<timestamp>.bak; second elevated pass is inert.
With the all-users attachment from inspect existing userScript assmb and see if it collect correct number of existing panels and buttons #9 already in place, and a
pre-existing %APPDATA% config that has clone-registry or
per-extension data the machine config lacks, launch Revit (or run
any pyrevit configs ... command) elevated and confirm the
per-user file gets retired to pyRevit_config.ini.split-admin.<timestamp>.bak. Run the same elevated command a second time and confirm no second retirement/backup happens.
🐛 BUG FOUND — pyrevit configs seed --lock correctly refuses writes, but gives no feedback either way.
From an elevated shell (seed writes to %ProgramData%, which needs
admin rights — a non-elevated attempt correctly failed with
Error: Failed seeding config file.):
```
pyrevit configs seed --lock
pyrevit configs rocketmode enable
```
Both commands returned no output at all — no confirmation, no
error, nothing. Whether they succeeded, failed silently, or did
nothing is impossible to tell from the CLI itself; had to fall back
to inspecting %ProgramData%\pyRevit\pyRevit_config.ini directly
(IsReadOnly attribute, grep for rocketmode) to know what actually
happened. A write command — especially one whose whole purpose is to
lock a file against future writes — should print something on
success (and definitely should say so if the second command was
silently refused). Not investigated further (root cause not chased).

Tier 4 — Regression check

PR claims 2 of 25 bundled extensions touch the config service directly
(beyond the stable user_config.* surface). Searched exhaustively —
open_config_file, coreutils.configparser, PyRevitConfig(,
config_service, IConfiguration, ConfigSections/ConfigSection( —
across every .py file in extensions/.

✅ Found one, confirmed identity: pyRevitTools.extension →
Legends.pulldown/FilterLegend.pushbutton/script.py, which calls
open_config_file(CONFIG_FILE) directly to manage its own separate
settings file (row/column sizing, sort order, auto-open behavior —
edited via its Shift-click config panel, legend_config.py). This is
the one real pre-existing extension at backward-compat risk from the
lower-level API changes.

✅ Exercised live — FilterLegend button and its Shift-click
legend_config.py settings panel both ran cleanly in Revit, saved/reloaded
without error. Confirms this extension's direct open_config_file usage
still works against the new config service.

ChrisCrosley added 22 commits June 21, 2026 17:10

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.

refactor(loader): drop the dead IniFile reader

7af7c46

PyRevitConfig now reads through the shared configuration service, so the Win32 INI reader is unused. Remove it and move the still-used PythonListParser into its own file.

jmcouffin requested a review from Copilot June 23, 2026 20:04

Copilot started reviewing on behalf of jmcouffin June 23, 2026 20:05 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

ChrisCrosley added 4 commits June 23, 2026 20:57

Merge branch 'config-store-phase1' into config-store-phase3

d875f6a

romangolev marked this pull request as ready for review August 20, 2026 16:43

romangolev requested review from Wurschdhaud, jmcouffin and sanzoghenzo August 20, 2026 17:13

jmcouffin requested a balanced review from Copilot August 20, 2026 18:11

Copilot AI reviewed Aug 20, 2026

View reviewed changes

Copilot started reviewing on behalf of jmcouffin August 20, 2026 21:28 View session

sanzoghenzo reviewed Aug 21, 2026

View reviewed changes

ChrisCrosley and others added 3 commits August 21, 2026 15:47

Update .gitignore

45b1bef

Co-authored-by: Andrea Ghensi <andrea.ghensi@gmail.com>

Update dev/pyRevitLabs.PyRevit.Runtime/ScriptConsole.cs

64780cc

Co-authored-by: Andrea Ghensi <andrea.ghensi@gmail.com>

Update dev/pyRevitLabs/pyRevitLabs.Configurations.Ini/IniConfiguratio…

7b857ae

…n.cs Co-authored-by: Andrea Ghensi <andrea.ghensi@gmail.com>

romangolev requested a review from dosymep August 23, 2026 17:42

dosymep approved these changes Aug 24, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

ChrisCrosley commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The service

The INI backend

Migration

Consumers

Behavior changes

Breaking changes

Testing

Out of scope

Uh oh!

devloai Bot commented Jun 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romangolev commented Aug 20, 2026

Uh oh!

jmcouffin commented Aug 20, 2026

Uh oh!

romangolev commented Aug 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

sanzoghenzo commented Aug 21, 2026

Uh oh!

romangolev commented Aug 21, 2026

Uh oh!

sanzoghenzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmcouffin commented Aug 21, 2026

Uh oh!

sanzoghenzo commented Aug 21, 2026

Uh oh!

jmcouffin commented Aug 21, 2026

Uh oh!

ChrisCrosley commented Aug 21, 2026

Uh oh!

jmcouffin commented Aug 21, 2026

Uh oh!

ChrisCrosley commented Aug 21, 2026

Uh oh!

ChrisCrosley commented Jun 23, 2026 •

edited

Loading

dosymep commented Aug 24, 2026 •

edited

Loading

romangolev commented Aug 24, 2026 •

edited

Loading