Skip to content
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

Require gperf at build-time #1870

Merged
merged 1 commit into from
Apr 28, 2024
Merged

Require gperf at build-time #1870

merged 1 commit into from
Apr 28, 2024

Conversation

brndnmtthws
Copy link
Owner

Rather than allowing the build to continue without gperf, we should fail with an error so that the colour behaviour does not change in a backward incompatible manner. The old colour behaviour should continue to work going forward.

This resolves #1868.

Rather than allowing the build to continue without gperf, we should
fail with an error so that the colour behaviour does not change in a
backward incompatible manner. The old colour behaviour should continue
to work going forward.

This resolves #1868.
@brndnmtthws brndnmtthws requested a review from Caellian April 28, 2024 20:55
@github-actions github-actions bot added sources PR modifies project sources build system related to build system (CMake) and/or building process/assumptions labels Apr 28, 2024
Copy link

netlify bot commented Apr 28, 2024

Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit dc84652
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/662eb7b95231bc0008cfc921

Copy link
Collaborator

@Caellian Caellian left a comment

Choose a reason for hiding this comment

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

Looks okay for the intent of the PR.

I'd personally add a build option to disable color name parsing (and gperf build dep) as well - I don't use color names because hex codes are supported and I believe a lot of others don't either, so it might be a good way to reduce binary size (both previous impl and gperf take up sizeable amount of space in .text).

@brndnmtthws brndnmtthws merged commit 10045ab into main Apr 28, 2024
62 checks passed
@brndnmtthws brndnmtthws deleted the require-gperf branch April 28, 2024 22:27
@brndnmtthws
Copy link
Owner Author

I'd personally add a build option to disable color name parsing (and gperf build dep) as well - I don't use color names because hex codes are supported and I believe a lot of others don't either, so it might be a good way to reduce binary size (both previous impl and gperf take up sizeable amount of space in .text).

That's reasonable, just need to maintain backward compatibility.

@brndnmtthws brndnmtthws added the bug related to incorrect existing implementation of some functionality label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug related to incorrect existing implementation of some functionality build system related to build system (CMake) and/or building process/assumptions sources PR modifies project sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: conky: can't parse hex color 'lightgrey'
2 participants