Skip to content

fix: reject name lookup options without symbol names - #6768

Closed
MrMha wants to merge 1 commit into
boykopovar:mainfrom
MrMha:fix/lookup-name-required-20261016
Closed

MrMha wants to merge 1 commit into
boykopovar:mainfrom
MrMha:fix/lookup-name-required-20261016

Conversation

@MrMha

@MrMha MrMha commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

What

A bare --name or --symbol was parsed as an empty list and fell through to the source scan/database path. Reject an explicitly requested name lookup without names, including abbreviated options, before accessing the database or source tree. The existing empty --nid filter behavior is preserved.

Tested

macOS 27.0.1 arm64, based on main 7dcfff559f68fc4ffb52f836134d3133492a3d00. 31 tests in the affected Python module pass on Python 3.12.14. The new regression also passes on Apple Python 3.9.6.

The new regression fails against the original behavior and passes with this fix. Whitespace and repository conventions checks pass. Full runtime/title execution and Windows/Linux execution were not performed locally.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; silent stubs are documented where required
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (OpenAI Codex)

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 6578 tools/nid_names.py
tools/tests/test_nid_names.py

@MarcZX10

Copy link
Copy Markdown
Contributor

For coordination with #6790: the same explicit-empty --name/--symbol inputs currently produce different outcomes. #6790 treats them as an empty selection and avoids database/source scanning; this PR returns a usage error. Could you and that author settle one CLI contract and add a test for the cross-case before either merges? I have not found a reason the empty case must be treated differently from the already-supported explicit-empty --nid case.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@blawesom

Copy link
Copy Markdown
Contributor

Rebase handoff + one test-suite finding — rebased onto current main (authorship unchanged). The conflict in tools/nid_names.py took your rejection path (--name/--symbol requires at least one symbol name).

The finding: your rejection breaks 8 of main's nid_database_cache tests (tools/tests/test_nid_names.py, 35 tests, 8 failures) — they invoke nid_names.py with --name in the empty/no-argument forms and expect success (returncode 0), which your parser.error turns into 2. Those tests were added on main after your branch point, exercising exactly the behaviour your change forbids. So the PR needs either the tests updated to expect the rejection (if the rejection is the intended behaviour — likely, given the PR's title) or the rejection scoped to the forms the tests don't cover. Everything else passes (1337/1339: those 8 plus the known main regression agc_driver_tessellation, #4727) and conventions are green.

To pick it up:

git fetch https://github.com/blawesom/AnyPS5 rebase/6768
git checkout <your branch>
git merge --ff-only FETCH_HEAD

Head is 057d69d29.

@MrMha MrMha closed this Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants