Skip to content

tool_urlglob: add named globs#21409

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/named-globs
Closed

tool_urlglob: add named globs#21409
bagder wants to merge 1 commit into
masterfrom
bagder/named-globs

Conversation

@bagder

@bagder bagder commented Apr 22, 2026

Copy link
Copy Markdown
Member

Idea-by: Bastian Jesuiter

Verified by test 2408 - 2411

@bagder bagder added cmdline tool feature-window A merge of this requires an open feature window labels Apr 22, 2026
@github-actions github-actions Bot added the tests label Apr 22, 2026
@bagder

bagder commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

Blurgh, using <> in the command lines for the tests makes the XML-checker unhappy...

@bagder bagder marked this pull request as ready for review April 22, 2026 11:53
@vszakats

Copy link
Copy Markdown
Member

Flaky Azure Ubuntu install failures should hopefully be gone after a rebase.

bagder added a commit that referenced this pull request Apr 25, 2026
Idea-by: Bastian Jesuiter

Verified by test 2408 and 2409

Closes #21409
@bagder bagder force-pushed the bagder/named-globs branch from 7a4dcf1 to 173d6d5 Compare April 25, 2026 21:28
@bagder bagder requested a review from Copilot April 25, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds support for named URL globs in the curl tool’s URL globbing parser, allowing glob components in a URL to be named and then referenced from --output using #<name>.

Changes:

  • Extend tool_urlglob patterns with an optional name and parse <name> after { / [.
  • Add output filename expansion support for #<name> references (in addition to #1, #2, ...).
  • Add new test cases (2408/2409) and document the new syntax.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/tool_urlglob.h Adds name field to URLPattern to store an optional glob name.
src/tool_urlglob.c Parses <name> in URL globs, expands #<name> in output templates, and frees allocated names.
tests/data/test2408 New test for named {...} globs referenced by name in -o.
tests/data/test2409 New test for named [...] globs referenced by name in -o.
tests/data/Makefile.am Registers the new tests in the test suite list.
docs/cmdline-opts/output.md Documents named glob references in --output and provides examples.
docs/cmdline-opts/_GLOBBING.md Documents named glob syntax in URLs and how output references work.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/cmdline-opts/_GLOBBING.md
Comment thread docs/cmdline-opts/output.md
Comment thread src/tool_urlglob.c Outdated
Comment thread src/tool_urlglob.c
Comment thread src/tool_urlglob.c
Comment thread docs/cmdline-opts/output.md Outdated
bagder added a commit that referenced this pull request Apr 26, 2026
Idea-by: Bastian Jesuiter

Verified by test 2408, 2409 and 2410.

Closes #21409
@bagder bagder force-pushed the bagder/named-globs branch from 173d6d5 to 43a4b31 Compare April 26, 2026 10:07
bagder added a commit that referenced this pull request Apr 26, 2026
Idea-by: Bastian Jesuiter

Verified by test 2408 - 2411

Closes #21409
@bagder bagder force-pushed the bagder/named-globs branch from c241951 to a9b998d Compare April 26, 2026 10:52
@bagder

bagder commented Apr 26, 2026

Copy link
Copy Markdown
Member Author

augment review

@augmentcode

augmentcode Bot commented Apr 26, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR adds support for named URL glob parts, allowing users to reference specific glob components when generating output filenames.

Changes:

  • Extends the URL glob pattern representation with an optional per-glob name field.
  • Updates glob parsing to accept an optional <name> immediately after { or [, and rejects duplicate names.
  • Adds a helper to look up patterns by name and reuses shared logic for printing detailed glob errors with caret positioning.
  • Enhances output filename expansion to support named references (#<name>) in addition to numeric references (#1, #2, ...).
  • Improves error reporting for output-glob expansion by printing the specific glob error and marking it as synthetic to avoid duplicate reporting.
  • Adds new tests (2408�02411) covering named globs in URLs, named output references, duplicate-name rejection, and referencing an unknown name.
  • Updates a few existing test expectations to match the adjusted error message wording (now �in position�).

Technical notes: Names are case-sensitive, length-limited, and are freed during glob cleanup; unknown named references produce a hard error.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread docs/cmdline-opts/output.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/tool_urlglob.c
Comment thread src/tool_urlglob.h Outdated
Comment thread src/tool_urlglob.c
bagder added a commit that referenced this pull request Apr 30, 2026
Idea-by: Bastian Jesuiter

Verified by test 2408 - 2411

Closes #21409
@bagder bagder force-pushed the bagder/named-globs branch from 0cfe637 to e4ee82e Compare April 30, 2026 11:15
Idea-by: Bastian Jesuiter

Verified by test 2408 - 2411

Closes #21409
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
Idea-by: Bastian Jesuiter

Verified by test 2408 - 2411

Closes curl#21409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmdline tool feature-window A merge of this requires an open feature window tests

Development

Successfully merging this pull request may close these issues.

3 participants