| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
The project name flowed into the generated config as name = "{name}"
with no escaping, so a name containing a quote produced a
pythonnative.toml that couldn't be parsed. display_name is derived from
the same string, and the commented url_schemes, bundle_id, and key_alias
examples interpolate it too.
_toml_escape walks the string once against a compact-escape map, so each
character is consulted exactly once and can't be re-escaped. Anything
else in TOML's must-escape ranges falls through to \uXXXX. Tab passes
through raw; U+000B does not, since TOML 1.0 gives it no compact escape.
render_default_toml is public API and reachable from library callers with
no validation in front of it, so escaping belongs at the render boundary
rather than relying on a CLI guard.
pn init now rejects a typed name that doesn't match ^[a-z][a-z0-9_-]*$, before touching the filesystem, and suggests a sanitized alternative. --force does not lift it. The charset check sits after the lexical path guard and before Path.cwd(), preserving the invariant that nothing touches the filesystem until the name is known good. fullmatch, not match: $ also matches immediately before a trailing newline, so match accepted "app\n" and created a directory whose name contained one, with and without --force. Validation applies to the typed name only. Applying it to the name derived from the current directory would break pn init in any directory not already lowercase-kebab, on a path where the user supplied nothing to correct. pn init "" no longer falls through to the no-name path and silently scaffolds into the current directory. The charset check tests name is not None rather than truthiness; the other guards are unchanged. The MyApp test fixture is renamed to my_app, which moves the paired app id assertions, including the relocated Android package path segments. Closes pythonnative#29
|
@owenthcarey following up on the two notes above (hyphen mapping and the long-name OSError) - happy to open issues for either if you'd like them tracked separately, otherwise no action needed on my end. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
What
pn init now rejects a typed project name that doesn't match ^[a-z][a-z0-9_-]*$, before
touching the filesystem, with a suggested legal alternative. render_default_toml()
TOML-escapes every interpolated string value.
Closes #29.
Why
The project name flowed straight into the generated config as name = "{name}", so a name
containing a quote produced a pythonnative.toml that couldn't be parsed. _app_id_from_name
separately stripped everything outside [a-z0-9_], so the directory name, the name field,
and the app id could end up representing three different things.
Following @owenthcarey's direction on the issue: validate in pn init, and escape in
render_default_toml() as defense in depth.
How (brief)
cwd = Path.cwd(), so Make pn init <name> create the project directory #21's invariant holds that nothing touches the filesystem until the
name is known good. --force does not lift it.
runs of illegal characters to a single _, strip leading and trailing _ and -, prefix
if it doesn't start with a letter, fall back to a fixed default. The suggestion is itself
always a legal name, asserted as a property over a 21,131-case corpus rather than a handful
of examples.
prefix, since nothing is being refused to overwrite, and because
test_cli_init_rejects_path_like_names keys on that prefix to distinguish a path rejection
from a charset one.
map, so each character is consulted exactly once and cannot be re-escaped. Anything else in
TOML's must-escape ranges falls through to \uXXXX. Tab passes through raw; U+000B does not,
since TOML 1.0 gives it no compact escape. Applied to all eight interpolation points,
including the commented url_schemes, bundle_id, and key_alias examples.
The commented url_schemes, bundle_id, and key_alias examples are covered by a test that
uncomments and parses them. tomllib never sees those lines otherwise, so a missing escape
there is invisible until a user uncomments one — verified by dropping quote and backslash
escaping on just those lines, which leaves all 32 other config tests green.
Validation is named-path only
Applying the regex to project_name = name or cwd.name was constructed and run: pn init with
no name inside a directory called MyProject exits 1 with Invalid project name: 'MyProject'.
Sampling ordinary directory names, MyProject, PythonNative, Documents, Desktop,
Projects, 2048, and My App all fail; only src, my_app, hello-world, e2e-suite
pass. That's a regression on a path where the user supplied nothing to correct, and the error
would be telling them to rename their working directory.
Your phrasing — "a rejected name should get an error suggesting a sanitized alternative" — only
makes sense for a name someone typed, so I've read it that way. Happy to flip it if you meant
both. test_cli_init_without_name_accepts_a_cwd_that_fails_the_pattern pins the behavior, since
the decision rests on it.
pn init "" is fixed here
It previously fell through to the no-name path and silently scaffolded into the current
directory, because every guard read if name and .... An empty string is invalid under the
pattern, so the charset check tests name is not None. The other two guards are untouched.
Escaping is defense in depth, not a live CLI path
After validation, no name the CLI accepts needs escaping. It still matters because
render_default_toml is public API, called directly from tests/project/test_config.py and
test_doctor.py and available to any library caller with no validation in front of it; and
because display_name is derived text — name.replace("_", " ").replace("-", " ").title() —
so its safety is a consequence of the charset rather than a property of the derivation. If the
charset ever widens, display_name breaks before name does. The escaping tests call the
function directly for that reason.
Testing
./scripts/check.sh passes end to end. mkdocs build --strict exits 0 from a clean site/.
New coverage: rejection and --force rejection parameterized over uppercase, a space, a quote,
a leading digit, a leading underscore, non-ASCII, a trailing newline, and the empty string, each
asserting a non-zero exit, the suggestion's presence, and that nothing was created; the
suggestion-legality property; suggestion stability for already-legal names; the no-name path
succeeding in a MyProject/ directory; nine parameterized escaping cases in
tests/project/test_config.py asserting the rendered config parses with tomllib and
round-trips exactly, including the three commented examples once uncommented.
Every new guard and escape was mutation-checked: reverting fullmatch to match, allowing
uppercase, dropping the empty-string case, removing backslash escaping, and skipping the
commented lines each fail tests.
test_cli_init_rejects_path_like_names passes unchanged, which is the proof that guard order is
right — placing the charset check first breaks six of its seven cases, because the message
stops saying "Refusing to".
Risks/Impact
Breaking change, as you accepted on the issue: names outside the set are now rejected. In-repo
fallout was one fixture, MyApp, across seven test sites and the getting-started walkthrough,
renamed to my_app. Note the app id also appears split into path segments for the relocated
Android package, not only in dotted form — the build.gradle assertion and the
.../com/example/my_app/... path both moved with it.
Non-ASCII display names remain available by editing display_name after init, as you noted.
Docs/Follow-ups
Updated docs/getting-started.md (the command, the cd, the prose, and the sample config's id
and name), the pn init bullet in docs/api/cli.md, and a new entry in
docs/meta/troubleshooting.md following that file's one-heading-per-error-string convention.
docs/guides/configuration.md:66 documents the config's name field and is deliberately
untouched — a hand-edited config can still hold anything, and escaping is what makes that safe.
Two things for you(@owenthcarey), neither changed here:
my-app gives com.example.myapp while my_app gives com.example.my_app
(a-b-c → com.example.abc). Hyphens are legal in your charset, so for those names the
directory, the name field, and the app id still represent three different things — the
complaint pn init doesn't escape the project name written into pythonnative.toml #29 opens with. It holds for underscores. A reverse-DNS segment can't contain a
hyphen so something must give, and dropping is defensible; mapping - to _ would keep all
three aligned.
docs/examples.md:21 is the one doc using a hyphenated name.
produce a traceback from target.is_symlink() in the Make pn init <name> create the project directory #21 containment guard, so this is
reachable on main today and the charset guard neither introduces it nor makes it worse.
Fixing it means picking a maximum name length, which felt like the same kind of product
decision the charset was. Happy to open an issue or take it in a follow-up.