akx
commented
4 months ago cc @zharktas, @tomasr8, @eemeli – please weigh in if you have ideas :)
Closed akx closed 3 months ago
akx
commented
4 months ago cc @zharktas, @tomasr8, @eemeli – please weigh in if you have ideas :)
codecov[bot]
commented
4 months ago Attention: Patch coverage is 91.30435% with 6 lines in your changes missing coverage. Please review.
Project coverage is 91.22%. Comparing base (
d3346ee) to head (a820cf5). Report is 2 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| babel/messages/frontend.py | 91.30% | 6 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
akx
commented
4 months ago Oh, and cc @miigotu too of course as the opener of #777 😄
tomasr8
commented
4 months ago Looks great! I was just wondering how you can specify ignore paths. Would this work?
[[babel.mappings]]
method = "ignore"
pattern = "test/*"Can the pattern be a list?
akx
commented
4 months ago Looks great! I was just wondering how you can specify ignore paths.
By not specifying them in a mapping at all... 😅 There's also the command-line --ignore-dirs option for wholesale directory ignoring.
But if your imagined use-case is "extract from all HTML files with jinja2, but never extract anything from test*.html", that would be a separate issue. Perfectly doable with a no-op extractor (that we don't yet have), since the list of mappings is in priority order, but that'd also work for .cfg style mapping files.
Can the pattern be a list?
Might as well support that too!
akx
commented
4 months ago I'm a bit tired so maybe I missed something, but there already exists a noop extractor -
extract_nothingso for example this cfg currently works as expected:
My bad, it's not you being tired. I had forgotten there's a special if method == 'ignore': case in extract_from_file that handles that (as well as the 'ignore': extract_nothing case in _BUILTIN_EXTRACTORS)...
akx
commented
4 months ago Entirely unsolicited,
Not at all, thank you for the input! Much appreciated.
but an option might be: ...
I'm not completely sold on that – feels like it becomes too easy to accidentally do [mappings.python] if you expect to only have a single mapping for python, and you'd then get a (possibly inscrutable) parsing type error.
I would also drop the babel. prefix for the babel.toml file -- it is needlessly explicit (see e.g. .ruff.toml which drops the full tool.ruff prefix).
That's probably a good idea.
AA-Turner
commented
4 months ago I'm not completely sold on that – feels like it becomes too easy to accidentally do
[mappings.python]if you expect to only have a single mapping forpython, and you'd then get a (possibly inscrutable) parsing type error.
I think this is a problem regardless. For example this PR adds a config file with one entry, [jinja2: **.html]. Someone without knowledge of TOML's quirks might write
[mapping]
method = "jinja2"
pattern = "**.html"which looks right, but would instantly fail. Worse would be:
[babel.mappings]
method = "genshi"
pattern = "**/templates/**.html"
include_attrs = ""
[babel.mappings]
method = "genshi"
pattern = "**/templates/**.txt"
template_class = "genshi.template:TextTemplate"
encoding = "latin-1"This is luckily an error in TOML, but still confusing. There should be a clear and early error message if isinstance(config['mapping'], dict) is True (or in my proposal, any(isinstance(k, dict) for k in config['mapping'].keys())).
I think with the clear error message (that I posit is needed regardless of the option taken), my proposal still has merit. However, it does have drawbacks as you note.
akx
commented
3 months ago @AA-Turner @tomasr8 Do you have the time to take another look at this? Thanks!
akx
commented
3 months ago Thank you for the wonderful comments and review! ❤️
This PR offers to implement part of https://github.com/python-babel/babel/issues/777.
Namely, following this PR you would be able to run
pybabel extract -F babel.toml– or evenpybabel extract -F pyproject.toml, as we already will supporttool.babelas well asbabelfor the namespace. (Auto-detecting apyproject.tomlinstead ofsetup.cfgorbabel.cfgis not implemented in this PR.)I'm requesting comments on the proposed TOML format, which borrows the idea of mappings-as-lists from Mypy's overrides configuration.