Remove dead pre-2021 HOST_APP version checks - #3559
Conversation
After dropping Revit 2013-2020, HOST_APP.is_newer_than checks against those versions (and equivalent is_older_than / version comparisons) are always true or always false. Keep the supported-API path and drop the legacy branches, including the unused MatchConfigWindowLegacy UI. Co-authored-by: Jean-Marc Couffin <jmcouffin@users.noreply.github.com>
![devloai[bot]](https://avatars.githubusercontent.com/in/886113?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary:
Removes dead pre-2021 HOST_APP version checks across pyrevitlib and bundled extensions, following the drop of Revit 2013–2020 in #3438. Unwraps always-true/always-false is_newer_than/is_older_than/version gates, keeping live gates (2022+, 2024+, 2026+) and is_newer_than(2021) without or_equal. Deletes the unused MatchConfigWindowLegacy UI (XAML + resource dictionaries) and updates the csproj and installer manifest.
Review Summary:
Verified all 36 changed files against IronPython 2.7 constraints, black formatting, and PEP 8 naming conventions. All removed version gates were confirmed dead given the 2021 minimum host. Unused HOST_APP/UI imports were correctly cleaned up; HOST_APP was kept where still needed (e.g., is_newer_than(2021) in Make Pattern, is_newer_than(2022) in Print Sheets). The MatchConfigWindowLegacy removal is complete across all artifacts. One low-severity comment: get_connected_circuits implicitly returns None despite a list docstring contract — not a regression but worth making explicit.
Suggestions
| x | ||
| for x in element.MEPModel.GetElectricalSystems() | ||
| if x.CircuitType in circuit_types | ||
| ] |
There was a problem hiding this comment.
The docstring (line 2103) declares the return type as list, but after unwrapping the version gate this function implicitly returns None when element.MEPModel is falsy or GetElectricalSystems() is empty. This matches the pre-PR behavior (both branches were inside the same if), so it's not a regression — but since you're touching this function, consider adding an explicit return [] after the if block so the actual return type matches the documented contract:
if element.MEPModel and element.MEPModel.GetElectricalSystems():
return [
x
for x in element.MEPModel.GetElectricalSystems()
if x.CircuitType in circuit_types
]
return []actions
Quick Fix: Apply a small focused fix for this specific issue (takes < 1min).
Feedback: Rate this comment to help me improve future code reviews:
- 👍 Good - Helpful and accurate
- 👎 Poor - Wrong, unclear, or unhelpful
- Skip if you don't have any strong opinions either way.
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up to #3438 (which dropped Revit 2013–2020 support). With a minimum supported host of Revit 2021, it removes HOST_APP.is_newer_than / is_older_than / HOST_APP.version comparisons in pyrevitlib and bundled extensions that are now always-true or always-false, keeping only the supported-API branch. It also deletes the obsolete MatchConfigWindowLegacy UI (XAML + localized resource dictionaries) and its packaging references.
Changes:
- Unwrap always-true gates (e.g.
is_newer_than(2015..2019)) to keep the modern-API path, and drop always-false branches (e.g.is_older_than(2018/2019),version <= 2020), removing now-unusedHOST_APPimports where appropriate. - Delete
MatchConfigWindowLegacy.xamland its 8 localized resource dictionaries, and always loadMatchConfigWindow.xaml; remove its entries frompyrevit.aipandpyRevitWPFForms.csproj. - Simplify core host logic in
pyrevit/__init__.py(subversion,build,proc_window,pretty_name) since 2021+ always has the modern APIs.
Verification highlights: all removed from pyrevit import HOST_APP imports were confirmed unused in their files; files that only changed code (ColorSplasher, Make Pattern, ribbon, query, Print Sheets, etc.) retain valid HOST_APP usage; the collapsed create_param_value_filter and get_crop_region logic is preserved correctly; the Manage Tags block removed was unreachable code after a return; and no references to the deleted legacy window remain anywhere.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| release/pyrevit.aip | Removes installer row for deleted MatchConfigWindowLegacy.xaml |
| dev/pyRevitWPFForms/pyRevitWPFForms.csproj | Removes Page include for deleted legacy XAML |
| pyrevitlib/pyrevit/init.py | Simplifies subversion/build/proc_window/pretty_name; updates docstring example |
| pyrevitlib/pyrevit/runtime/init.py | Unconditionally adds AvalonDock reference |
| pyrevitlib/pyrevit/revit/tabs.py | Unwraps always-true 2018 gate in init_doc_colorizer |
| pyrevitlib/pyrevit/revit/db/update.py | Simplifies set_name/toggle_category_visibility/set_crop_region; drops HOST_APP import |
| pyrevitlib/pyrevit/revit/db/query.py | Simplifies get_name/get_connected_circuits/get_fillpattern/get_crop_region |
| pyrevitlib/pyrevit/revit/db/create.py | Simplifies view naming and create_param_value_filter rule handling |
| pyrevitlib/pyrevit/revit/db/init.py | Removes obsolete pre-2017 comment |
| pyrevitlib/pyrevit/loader/sessioninfo.py | Always sets app version env var from subversion |
| pyrevitlib/pyrevit/labs.py | Removes pre-2019 binding resolver activation and HOST_APP import |
| pyrevitlib/pyrevit/coreutils/ribbon.py | Defaults tooltip video to .mp4; unwraps split-button 2017 guard |
| pyrevitlib/pyrevit/coreutils/applocales.py | Always appends English_GB locale |
| extensions/.../Wipe Data Schema/script.py | Always uses doc.EraseSchemaAndAllEntities |
| extensions/.../Reload Links/script.py | Always includes CAD Links and shows selector |
| extensions/.../Override VG/script.py | Uses only modern foreground-pattern API |
| extensions/.../ColorizeGroupTypes/script.py | Uses only modern override-graphics API |
| extensions/.../Make Pattern/script.py, patmaker.py | Unwrap 2014/2018 gates for scale text and filled region |
| extensions/.../Match/script.py, config.py | Use only modern pattern APIs; always load non-legacy window |
| extensions/.../Match/MatchConfigWindowLegacy*.xaml | Deletes legacy window and localized resource dictionaries |
| extensions/.../Remove Underlay/script.py | Always uses SetUnderlayRange |
| extensions/.../Find Views By Filter/script.py | Uses only modern underlay-parameter path |
| extensions/.../Copy Views/script.py | Always uses ActiveGraphicalView path |
| extensions/.../Print Sheets/script.py | Defaults export encoding to utf_8 |
| extensions/.../ColorSplasher/script.py | Always enables background-pattern option |
| extensions/pyRevitTags.../Manage Tags/script.py | Removes unreachable post-return version branch |
| extensions/pyRevitTags.../lib/tagsmgr.py | Inlines fabrication centerline categories into skip list |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
bundle.yaml files with the 'min_revit_version' flag set can also be updated. Would fit into this PR, as it also touches tools, not .cs |
bundle.yaml minimums of 2014/2016/2019 are always satisfied now that pyRevit requires Revit 2021+. Leave 2022+ gates and the empty test bundle schema keys unchanged. Co-authored-by: Jean-Marc Couffin <jmcouffin@users.noreply.github.com>
Remove the pre-2017 version branch in GetUIApplication along with the now-unused NEW_UIAPP_FIELD_VERSION and LEGACY_UIAPP_FIELD constants, since the minimum supported Revit version no longer requires the legacy m_application field lookup.
Strip REVIT2013..REVIT2020 preprocessor checks and their fallbacks across the pyRevitLabs.PyRevit.Runtime project since those Revit versions are no longer supported, and tighten the loader theme detector's UIThemeManager gate to REVIT2021+.
|
@jmcouffin found some more dead code, committed here |
So that's this stuff cursor mentioned? |
|
@Wurschdhaud oops, my bad. I'm going to revert them now and open separate PR for that. I think we after that it's going to be safe to mark this PR as a ready for review |
Wurschdhaud
left a comment
Wurschdhaud
left a comment
There was a problem hiding this comment.
Tested some of them, didn't manage to break it. LGTM
|
📦 New work-in-progress (wip) builds are available for 7.0.0 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
|
📦 New work-in-progress (wip) builds are available for 7.0.0.26237+2139 |
Follow-up to #3438 (drop Revit 2013–2020): unwrap Python host-version gates that are now always true or always false.
HOST_APP.is_newer_than(v)ishost > v. With a minimum host of 2021, checks against 2013–2020 (andis_newer_than(2021, or_equal=True)) no longer branch. This PR keeps the remaining supported-API path and deletes the unused MatchConfigWindowLegacy UI.Description
Removes dead
HOST_APP.is_newer_than/is_older_than/ equivalentHOST_APP.versioncomparisons inpyrevitliband bundled extensions. Live gates for 2022+, 2024+, and 2026+ are unchanged.is_newer_than(2021)withoutor_equalis still live (false on 2021, true on 2022+). Those call sites were left as-is.Also removes redundant
min_revit_versionkeys frombundle.yamlfiles when the floor is 2014/2016/2019 (always satisfied on Revit 2021+). 2022+ minima and the empty test-bundle schema keys are unchanged.Checklist
Related Issues
Follow-up to #3438, as noted in #3438 (comment)
Additional Notes
Quoted related leftovers not in this PR (C# preprocessor, still-live Python gates):
#if REVIT2013…REVIT2020branches indev/pyRevitLabs.PyRevit.Runtime/(EventTelemetry.cs,EventHandling.cs,EventHooks.cs,ScriptCommands.cs,ScriptConsoleUtils.cs,CLREngine.cs,ContentEngine.cs,GrasshopperEngine.cs, plusRevitThemeDetector.cs). Those can be a separate cleanup.is_newer_than(2021)(2022+),2022/2022, or_equal=True,2024,2026,is_older_than(2023),is_older_than(2027),is_exactly(2021)/is_exactly('2022').min_revit_versionvalues that still hide tools on older hosts: 2022, 2023, 2025, 2027.docs/llms-full.txtstill mirrors the old snippets and should be regenerated, not edited by hand.