Skip to content
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.

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

files.Block: fixed couple of bugs, added try_prevent_shell_expansion and support for content: list[str] #1030

Merged
merged 6 commits into from
Jan 13, 2024

Conversation

stone-w4tch3r
Copy link
Contributor

@stone-w4tch3r stone-w4tch3r commented Nov 11, 2023

#1028
Removed dollar signs, looks like they were typo
#984
As #984 (comment), renamed files.marked_block in documentation to files.block
#1031
Adjusted awk to prevent duplications
#1029
Added try_prevent_shell_expansion parameter to partially help with mentioned issue

And also changed type for content to str | list[str]. This makes it more unified with facts.files.Block, that already returns list[str]

@stone-w4tch3r stone-w4tch3r changed the title files.Block: fixed couple of bugs, added try_prevent_shell_expansion and support for content: list[str] *files.Block:* fixed couple of bugs, added try_prevent_shell_expansion and support for content: list[str] Nov 11, 2023
@stone-w4tch3r stone-w4tch3r changed the title *files.Block:* fixed couple of bugs, added try_prevent_shell_expansion and support for content: list[str] files.Block: fixed couple of bugs, added try_prevent_shell_expansion and support for content: list[str] Nov 11, 2023
@@ -1661,7 +1666,7 @@ def block(
)

# have two entries in /etc/host
files.marked_block(
files.block(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1614,6 +1614,7 @@ def block(
line=None,
backup=False,
escape_regex_characters=False,
try_prevent_shell_expansion=False,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1029
helps with mentioned issue

@@ -1748,21 +1773,30 @@ def block(
f"{print_after} f!=1 && /{regex}/ {{ print x; f=1}} "
f"END {{if (f==0) print ARGV[2] }} {print_before}'"
)
cmd = StringCommand(out_prep, prog, q_path, f'$"{the_block}"', "> $OUT &&", real_out)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

...f'$"{the_block}"'...
Removed $

)
cmd = StringCommand(
out_prep,
prog,
q_path,
'$"' + content + '"',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

'$"' + content + '"',
Removed $

): # marked_block found but text is different
prog = (
'awk \'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=""}}'
f"/{mark_1}/ {{print; print x; f=0}} /{mark_2}/ {{print; f=1}} f'"
f"/{mark_1}/ {{print; print x; f=0}} /{mark_2}/ {{print; f=1; next}} f'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1031
Fixes this issue

@stone-w4tch3r stone-w4tch3r marked this pull request as ready for review November 11, 2023 13:23
Copy link
Member

@Fizzadar Fizzadar left a comment

Choose a reason for hiding this comment

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

@stone-w4tch3r huge thank you for working on this, I have to admit I have been struggling greatly to wrap my head around this operation, hugely appreciate your work here. Finally have pushed up some minor changes and fixed the tests to get this in.

@Fizzadar
Copy link
Member

(force push to get github to actually run actions, being very flakey at the moment)

@Fizzadar Fizzadar enabled auto-merge (squash) January 13, 2024 10:40
@Fizzadar Fizzadar merged commit 6972e3d into pyinfra-dev:2.x Jan 13, 2024
23 checks passed
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