Skip to content

Conversation

alexandear
Copy link
Member

The PR refactors the implementation of an unexported function cutVal in the printers package.

@ldez ldez added the declined label Dec 8, 2024
@ldez
Copy link
Member

ldez commented Dec 8, 2024

A character can use several runes so you cannot do that.
This is why utf8.DecodeRuneInString is used.

@ldez ldez closed this Dec 8, 2024
@alexandear
Copy link
Member Author

A character can use several runes so you cannot do that. This is why utf8.DecodeRuneInString is used.

Does this mean that the TestTeamCity_cutVal is wrong? Because it is not failing on PR's changes.

https://github.com/golangci/golangci-lint/blob/v1.62.2/pkg/printers/teamcity_test.go#L95-L99

@ldez
Copy link
Member

ldez commented Dec 8, 2024

Does this mean that the TestTeamCity_cutVal is wrong?

No, it means that a test is validating only the cases defined inside the tests.

The current implementation of cutVal has some limitations too.

In fact, the current implementation and your proposal have the same limitations, so we can accept your PR.

For the details, emoji ZWJ sequences are not handled correctly, so a composite emoji can be cut improperly, but I think this is a minor edge case.

@ldez ldez reopened this Dec 8, 2024
@ldez ldez added topic: cosmetic changes contain cosmetic improvements and removed declined labels Dec 8, 2024
@ldez ldez added this to the next milestone Dec 8, 2024
@ldez ldez merged commit 127edc0 into golangci:master Dec 8, 2024
28 checks passed
@alexandear alexandear deleted the dev-simplify-cutval branch December 9, 2024 08:14
@ldez ldez modified the milestones: next, v1.63 Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: cosmetic changes contain cosmetic improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants