Skip to content

refactor: declare the names the interactive shell exposes - #59

Merged
cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:refactor/declare-shell-globals
Sep 13, 2026
Merged

cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:refactor/declare-shell-globals

Conversation

@blaipr

@blaipr blaipr commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

main() declares global session_args, global root and global v2 and assigns them, which is how ascender-shell ends up with a connected root and v2 at the interactive prompt. None of the three has a module-level binding, so global creates them on first assignment.

That works, and it means the module does not say what it exposes. Reading the file top to bottom gives no indication that root and v2 are the point of the script, and importing it before main() runs raises AttributeError for names that are, in every practical sense, its public surface.

Declared at module scope as None, with a comment saying what fills them. main() keeps its global statements and assigns exactly as before, so the only change is that the names exist, holding None, before it runs. That is also what the type checker's three unresolved-global warnings were pointing at.

Verified with black --check, flake8, the unit suite at 355 passing, and importing the module without calling main(), which now reports None None None instead of raising.

@ciq-it-service-account

ciq-it-service-account commented Sep 12, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

`main()` declares `global session_args`, `global root` and `global v2` and assigns them, which is how `ascender-shell` ends up with a connected `root` and `v2` at the interactive prompt. None of the three has a module-level binding, so `global` creates them on first assignment.

That works, and it means the module does not say what it exposes. Reading the file top to bottom gives no indication that `root` and `v2` are the point of the script, and importing it before `main()` runs raises `AttributeError` for names that are, in every practical sense, its public surface.

Declared at module scope as `None`, with a comment saying what fills them. `main()` keeps its `global` statements and assigns exactly as before, so the only change is that the names exist, holding `None`, before it runs. That is also what the type checker's three `unresolved-global` warnings were pointing at.

Verified with `black --check`, `flake8`, the unit suite at 355 passing, and importing the module without calling `main()`, which now reports `None None None` instead of raising.
@blaipr
blaipr force-pushed the refactor/declare-shell-globals branch from 030a2d7 to e8517b7 Compare September 13, 2026 09:03
@cigamit cigamit self-assigned this Sep 13, 2026
@cigamit cigamit added the enhancement New feature or request label Sep 13, 2026
@cigamit
cigamit merged commit 6f58073 into ctrliq:main Sep 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

3 participants