Skip to content

Conversation

@radarhere
Copy link
Member

No description provided.

Comment on lines 222 to 224
} else if (strcmp(PILmode, "RGBA;16B") == 0) {
return TYPE_RGBA_16;
} else if (strcmp(PILmode, "CMYK") == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

More of a style issue: these all return, I think it reads a little cleaner to remove the elses:

Suggested change
} else if (strcmp(PILmode, "RGBA;16B") == 0) {
return TYPE_RGBA_16;
} else if (strcmp(PILmode, "CMYK") == 0) {
}
if (strcmp(PILmode, "RGBA;16B") == 0) {
return TYPE_RGBA_16;
}
if (strcmp(PILmode, "CMYK") == 0) {

etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I've updated the commit.

// LabX equivalent like ALab, but not reversed -- no #define in lcms2
return (COLORSPACE_SH(PT_LabV2) | CHANNELS_SH(3) | BYTES_SH(1) | EXTRA_SH(1));
}
else {
Copy link
Member

Choose a reason for hiding this comment

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

And ditch and dedent this block?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I know what you meant, I've updated the commit.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, like that :)

@aclark4life
Copy link
Member

Speaking of formatting, anyone interested in adding clang-format in some practical way? Maybe one time, since C is not touched very often. 🤔

@hugovk
Copy link
Member

hugovk commented Apr 25, 2024

Yes, like #4493 / #4770 ;)

But looks like we forgot to add it to pre-commit/CI and there's some new formatting that can be applied.

@aclark4life
Copy link
Member

Hmmm, I ran it local and got a bunch of additional changes but … cool! Thanks

@hugovk hugovk merged commit 3823675 into python-pillow:main Apr 25, 2024
@radarhere radarhere deleted the imagingcms branch April 25, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants