Skip to content

Conversation

@cofinalsubnets
Copy link
Contributor

Adds command line flags to sed to require strict POSIX behavior. Extended regexes are still used by default but an ignored -E flag is added for compatibility with other sed versions.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Nov 25, 2025
Copy link
Member

@alimpfard alimpfard left a comment

Choose a reason for hiding this comment

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

Please squash all the fixup commits into the main one, something something commit guidelines.

Comment on lines 874 to 878
ByteString result;
if (s_args.regex.has<Regex<PosixExtended>>()) {
result = s_args.regex.get<Regex<PosixExtended>>().replace(pattern_space_sv, s_args.replacement, s_args.options);
} else {
result = s_args.regex.get<Regex<PosixBasic>>().replace(pattern_space_sv, s_args.replacement, s_args.options);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ByteString result;
if (s_args.regex.has<Regex<PosixExtended>>()) {
result = s_args.regex.get<Regex<PosixExtended>>().replace(pattern_space_sv, s_args.replacement, s_args.options);
} else {
result = s_args.regex.get<Regex<PosixBasic>>().replace(pattern_space_sv, s_args.replacement, s_args.options);
}
auto result = s_args.regex.visit([&](auto& re) { return re.replace(pattern_space_sv, s_args.replacement, s_args.options); });

Adds command line flags to `sed` to require strict POSIX behavior.
Extended regexes are still used by default but an ignored -E flag is
added for compatibility with other sed versions.
@alimpfard
Copy link
Member

Thank you!

@alimpfard alimpfard merged commit 5abb638 into SerenityOS:master Nov 29, 2025
12 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Nov 29, 2025
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