Skip to content

Conversation

@Un1q32
Copy link
Contributor

@Un1q32 Un1q32 commented Aug 17, 2023

sed -r and -E do the same thing, but sed -E is the portable version that works with all versions of sed, however some old versions of GNU sed only support -r, a workaround is to check if we're running GNU sed, and use -r if we are

@Un1q32
Copy link
Contributor Author

Un1q32 commented Sep 21, 2023

bump

Copy link
Collaborator

@catumin catumin left a comment

Choose a reason for hiding this comment

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

Built a version of GNU sed from before -E was added (I used 4.1.5, it was added 4.2), and then tested with that sed, my system's recent GNU sed, and macOS sed. All behaved as they should.

@Un1q32
Copy link
Contributor Author

Un1q32 commented Sep 26, 2023

@BKasin can you try the latest commit, I reworked it into a wrapper function that replaces the arguements if sed is GNU sed, should be completely transparent so nobody has to remember to add $sed_ext to every sed command from now on

@catumin
Copy link
Collaborator

catumin commented Sep 26, 2023

Will do.

Copy link
Collaborator

@catumin catumin left a comment

Choose a reason for hiding this comment

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

Re-tested on previously stated platforms and can confirm the command gets rewritten properly based on GNU vs non-GNU sed. Screenshot shows that detection of GNU sed worked, and the ASCII art printing means that the sed on line 5222 works.

sed

@hykilpikonna hykilpikonna merged commit b1df896 into hykilpikonna:master Sep 26, 2023
@hykilpikonna
Copy link
Owner

Nice fix, thanks!

And @BKasin thank you so much for the detailed testing!

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.

3 participants