Skip to content

Add disabled_converters support for built-ins#1686

Open
txhno wants to merge 1 commit into
microsoft:mainfrom
txhno:fix-disabled-builtins
Open

Add disabled_converters support for built-ins#1686
txhno wants to merge 1 commit into
microsoft:mainfrom
txhno:fix-disabled-builtins

Conversation

@txhno

@txhno txhno commented Apr 10, 2026

Copy link
Copy Markdown

Closes #1665.

Adds a disabled_converters option to built-in registration and covers it with focused tests.

Validation:

  • pytest tests/test_module_misc.py -k "disabled_converters or enable_builtins_with_disabled_converters"
  • direct runtime check in the repo venv

@VANDRANKI VANDRANKI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for implementing this. The approach is clean and addresses what the issue asked for.

maybe_register helper - good pattern. Keeps the registration block readable without repeating the guard on every line.

Class name strings - type(converter).__name__ not in disabled_converters is the right check. Callers do not need to import converter classes, which was the core ask.

Priority refactor - I checked: register_converter defaults to PRIORITY_SPECIFIC_FILE_FORMAT, so assigning it explicitly to RssConverter, WikipediaConverter, etc. only makes the implicit default visible. No behavioral change.

Tests - both paths covered: MarkItDown(disabled_converters=...) constructor and enable_builtins(disabled_converters=...) directly. The registered_converter_names helper is reusable.

LGTM.

@txhno

txhno commented Apr 13, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@txhno

txhno commented Apr 13, 2026

Copy link
Copy Markdown
Author

CLA is complete from my end now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No way to disable specific built-in converters without subclassing

3 participants