Skip to content

feat(pathops): support line replacements and appending in ensure_contents - #676

Open
TheJJ wants to merge 1 commit into
canonical:mainfrom
TheJJ:file-content-editing
Open

TheJJ wants to merge 1 commit into
canonical:mainfrom
TheJJ:file-content-editing

Conversation

@TheJJ

@TheJJ TheJJ commented Sep 9, 2026

Copy link
Copy Markdown
Member

Add matcher and replace arguments to update matching lines in an existing file instead of requiring the full contents.

This allows arbitrary search and replaces in a file, e.g. to

  • set a value to a conf file key
  • append the key-value if the conf file key is missing
  • do backreferencing to parts of the match regex
  • do multiline replaces in a file

@james-garner-canonical james-garner-canonical left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this idea, Jonas. Could you please give some examples of the use cases for these two features?

@TheJJ
TheJJ force-pushed the file-content-editing branch from a8612e5 to 9c5c593 Compare September 10, 2026 12:54
…ents

Add matcher argument to update matching lines in an existing file instead of requiring the full contents.
This supports backreferences, and configurable handling when there's no match.

A str matcher is compiled with re.MULTILINE; pass a precompiled re.Pattern for other flags.
@TheJJ
TheJJ force-pushed the file-content-editing branch from 9c5c593 to af3553b Compare September 10, 2026 12:54
@TheJJ

TheJJ commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

Sure - i would mainly use it for line replacements (or append if missing) in config files.

# set a value in a config file (replaces every matching line)
ensure_contents(path, 'DEBUGINFOD_PORT=8002', matcher=r'^DEBUGINFOD_PORT=')

# same, but append the line if the key is absent
ensure_contents(path, 'DEBUGINFOD_PORT=8002', matcher=r'^DEBUGINFOD_PORT=', no_match='append')

# rewrite the whole file if the key is absent
ensure_contents(path, 'server_name=example.com', matcher=r'^server_name=', no_match='replace')

# backreferences: keep the value, add a suffix
ensure_contents(path, r'MAX_TIME=\1  # juju managed', matcher=r'^MAX_TIME=(\d+)$')

# named groups
ensure_contents(path, r'user=\g<name> (juju-managed)', matcher=r'^user=(?P<name>\S+)$')

# multi-line block: precompiled pattern with DOTALL
ensure_contents(path, '# BEGIN MANAGED\nnew=3\n# END MANAGED',
                matcher=re.compile(r'# BEGIN MANAGED.*# END MANAGED', re.DOTALL))

# whole-file anchor: rewrite only the first line (no MULTILINE)
ensure_contents(path, '#!/usr/bin/env python3', matcher=re.compile(r'^.*$'))

@james-garner-canonical james-garner-canonical left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for explaining the use cases @TheJJ. I also had a look at the linked PR.

I don't think we should add this functionality to ensure_contents directly, for two reasons:

  1. We hope ensure_contents is widely used as-is, so it would be nice to keep that highly-used function's implementation (and signature) as simple as possible.
  2. I'm worried that it will encourage an anti-pattern: calling ensure_contents multiple times for the same file. This is an anti-pattern because every ensure_contents requires at least one file read (possibly from a sidecar container), and call resulting in a change also requires a write (possibly to a sidecar container).

I'm not opposed to adding a separate function though, that would address both of these concerns. But it seems like it requires a bit of design to get right and continue to address the use case you're targeting.


For example, extending this PR's design with source and matcher, maybe the function should take a Mapping of matchers to replacements -- though it would be more elegant if the replacement was more than just a simple string, so maybe it should actually be an Iterable of regex substitutions (and we only write back if the contents actually change after applying the regex (or use re.subn to check explicitly)).

But I guess each substitution also needs a no_match case ... probably str | None with str meaning 'append this string if there's no match' and None meaning 'ignore if there's no match'. And if the target file doesn't exist (or is empty), we're just applying all the no_match cases in sequence. So maybe a Mapping of regex pattern to no_match behaviour is the right shape, and we'd have something like replace_contents(path, replacements, *, mode, user, group) -> bool.

Just thinking out loud, LMK if you think a design like this would be helpful for your use cases or if you have other ideas in mind. I'll need to pitch the design to the team as well (just because it's new ongoing maintenance surface for us).

@james-garner-canonical

Copy link
Copy Markdown
Collaborator

Though I guess a substitution kind of needs to be a pattern and replacement pair anyway so maybe a mapping is a bad idea and we'd really want a sequence of two tuples (skip if no match) or three tuples (specify behaviour if no match). But maybe we also need to capture stuff like count or regex flags for each replacement too so we end up needing a real structured object ...

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.

2 participants