Skip to content

Conversation

@izabera
Copy link
Contributor

@izabera izabera commented Jan 26, 2024

posix requires base 10 numbers, so 010 should equal 10 and not 8

posix requires base 10 numbers, so 010 should equal 10 and not 8
@landley
Copy link
Owner

landley commented Jan 26, 2024

Posix doesn't allow + on address ranges either, we're already doing extensions and have a "deviations from posix" section starting on line 16. Do you have a script that broke, or is this a speculative patch?

@izabera
Copy link
Contributor Author

izabera commented Jan 27, 2024

no i don't have a real world script that broke, i found this accidentally when looking at how a few different sed implementations parsed other things. it's very unimportant
i would argue that this is not an extension the same way that adding support for + was, as + would have no meaning there in posix sed. 010 does have a meaning and it's 10
but yes, i see now that there's a couple of intentional deviations from posix already. either way is fine i guess

@landley
Copy link
Owner

landley commented Jan 27, 2024

It's mostly hex ala 0xf I was thinking of. 0 isn't a valid line number (gsed complains!) and x can't take arguments, so that can't be mistaken for an existing sed command.

You're right that there might be a sed script out there this breaks, but in the absence of finding a package build or similar doing it I'm torn.

The problem is, when I fix this kind of thing I want to add a patch to sed.tests to distinguish the fixed vs non-fixed behavior, and if I were to add a sed -n '02p' patch to my sed.tests and then upgrade debian to a new version, that's the kind of persnickety corner case test that seems likely to break on TEST_HOST because THEY decided to support multiple bases in some new release. (Or just always error on leading 0, the way it currently errors on 0 result). Meaning I wouldn't want to add that test if I wasn't changing the behavior, so... should I change the behavior?

Hence the indecision.

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