Skip to content

Conversation

@kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Dec 28, 2025

Description

This PR covers a few minor areas:

Fixes for the SettingsQR Generator

SettingsConstants.get_detected_languages() scans the translation dir to see which languages are on board. But its getcwd() approach doesn't work in the SettingsQR Generator because the generator runs from a different shell context. Changing its directory reference to __file__ guarantees that we can find our way to the translations subdir no matter which dir the instigating shell command is run from.

Remove if __name__ == "__main__":

The SettingsQR Generator was using this python convention to run the settings_definition.py file directly to extract the SettingsDefinition. But elsewhere we have preferred to avoid providing direct execution of individual source files. The SettingsQR Generator has been refactored so that this direct execution is no longer necessary.

see: (pending PR)

Improve test maintainability

In #844 I struggled to update the SettingsQR tests correctly because they use pasted-in config strings:

settingsqr_data = f"""settings::v1 name={ settings_name.replace(" ", "_") } persistent=D coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss,ms scripts=nat,nes,tr xpub_details=E passphrase=E camera=180 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E"""

This PR refactors the tests to provide a default config string that is generated directly from the current SettingsDefinition, basically eliminating future maintenance issues. This is facilitated by a minor refactor in SettingsDefinition.get_defaults().

Also guarantees that SettingsEntry.abbreviated_name will always have a value (is now set to attr_name if none is provided). This was for convenience to avoid repeating this over and over again in the SettingsQR tests:

attr_name = settings_entry.abbreviated_name or settings_entry.attr_name

Includes other minor maintainability edits.

Also anticipates the Settings change in #844 and changes the test that had been referencing the soon-to-be-removed "Coordinators" setting.


Testing

Really just need to code review the changes to the tests and verify that they still pass. And then verify that we can still detect which languages are onboard when running the code.

The effect on the SettingsQR Generator is a separate matter that can be tested in its PR after this change is merged.


This pull request is categorized as a:

  • Code refactor

Checklist

  • I’ve run pytest and made sure all unit tests pass before submitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • N/A

I have tested this PR on the following platforms/os:

@kdmukai kdmukai force-pushed the 2025-12_settings_cleanup_v2 branch from 9b032f0 to ebfc6b4 Compare January 20, 2026 16:29
@kdmukai
Copy link
Contributor Author

kdmukai commented Jan 20, 2026

Rebased to current dev.

@newtonick newtonick removed the merge conflicts has merge conflicts that need resolution label Jan 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 0.8.7 Needs Code Review

Development

Successfully merging this pull request may close these issues.

2 participants