Skip to content

Allows for multiple -s|--section options #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
caruccio wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Allows for multiple -s|--section options #148

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe0decb
Allows for multiple -s|--section options
caruccio Aug 4, 2026
aca5157
Merge sections, ensuring multiple values are all iterables
caruccio Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions jinja2cli/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,13 +551,22 @@ def cli(opts: argparse.Namespace, args: Sequence[str]) -> int:
ext = f"jinja2.ext.{ext}"
extensions.append(resolve_extension(ext, os.getcwd()))

# Use only a specific section if needed
if opts.section:
section = opts.section
# Use specified sections if needed
if len(opts.section) == 1:

@mattrobenolt mattrobenolt Aug 11, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This crashes when -s isn't passed at all. action="append" without a default leaves opts.section as None, and len(None) is why CI is fully red right now. The failing tests never pass -s, so the default path is just dead. default=[] on the argument fixes it.

section = opts.section[0]
if section in data:
data = data[section]
else:
raise InvalidUsage(f"unknown section: {section}")
elif len(opts.section) > 1:
# for multiple values, all must be iterables
merged_data = {}
for k, v in data.items():

@mattrobenolt mattrobenolt Aug 11, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Walk opts.section instead of data.items(). Two problems as written: a typo'd section name silently succeeds (single -s raises unknown section, multi just renders with the data missing), and merge order follows the data file's key order instead of the command line, so -s a -s b and -s b -s a produce identical output when keys collide. Iterating the flags fixes both.

if k in opts.section:
if not isinstance(v, Iterable):

@mattrobenolt mattrobenolt Aug 11, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Strings are Iterable, so this guard doesn't guard anything. A section whose value is a string sails through this check, then dict.update() explodes with ValueError: dictionary update sequence element #0 has length 1; 2 is required. You want collections.abc.Mapping here, and the error message means mapping, not iterable.

raise InvalidUsage(f"invalid section: {k}: must be iterable")
merged_data.update(v)
data = merged_data

deep_merge(data, parse_kv_string(opts.D or []))

Expand Down Expand Up @@ -702,8 +711,9 @@ def run() -> int:
parser.add_argument(
"-s",
"--section",
help="Use only this section from the configuration",
help="Use only this section from the configuration. Can be used multiple times.",
dest="section",
action="append",
)
parser.add_argument(
"--strict",
Expand Down
Loading