Skip to content

tool: cfg.GroupSubdirBy - group by dir or fname - #926

Merged
xushiwei merged 2 commits into
goplus:mainfrom
xushiwei:q
Oct 5, 2026
Merged

xushiwei merged 2 commits into
goplus:mainfrom
xushiwei:q

Conversation

@xushiwei

@xushiwei xushiwei commented Oct 5, 2026

Copy link
Copy Markdown
Member

No description provided.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: add GroupSubdirBy config option

The change is small, focused, and clean. Replacing the goFileOf bool parameter with a named iota enum (groupSubdirByNone/Dir/Fname) reads clearly, the empty-string + GroupSubdir=true backward-compat path is handled explicitly, and the default warning on bad input is a sensible defensive choice. No performance or clearly-exploitable security concerns were found.

A few points below are worth confirming or tightening before merge — mostly documentation clarity and one backward-compat edge case.

Minor notes (not inlined):

  • No unit tests exercise goFileOf's three branches or the config-to-enum switch. Given the "fname" collision semantics and the pos > 0 vs pos >= 0 asymmetry, a small table-driven test (top-level file, nested file, shared basename, leading-_ file, extensionless file × each enum value) would cheaply lock in intended behavior.
  • The accepted values "dir"/"fname" live only as inline string literals; defining them as named constants shared by the switch, the warning message, and the doc comment would avoid future drift.

Comment thread tool/gen.go
Comment thread tool/gen.go
Comment thread tool/config.go Outdated
Comment thread tool/config.go
@xushiwei
xushiwei merged commit ac230af into goplus:main Oct 5, 2026
2 checks passed
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.

1 participant