Skip to content

dev: simplify cutVal implementation #5206

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

Merged
merged 1 commit into from
Dec 8, 2024

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