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

Add JPEG XL support. #188

Merged
merged 4 commits into from
Jan 23, 2024
Merged

Add JPEG XL support. #188

merged 4 commits into from
Jan 23, 2024

Conversation

oupson
Copy link
Contributor

@oupson oupson commented Jan 22, 2024

Hi !

This PR add JPEG XL support for chafa using libjxl.

I'm not familiar with autotools so I hope the config is correct.

@hpjansson
Copy link
Owner

Thanks a lot! I can take the PR as-is, just a couple of nitpicks that you may want to address first:

Ideally, jxl-loader.[ch] should both have the license blurb with (C) your real name, if you care. If you don't, the copyright will still be yours, it'd just be harder to find you if we ever want to relicense (IIRC the only way to legally transfer copyright is with paperwork™). Your choice, though.

You may want to use the same indenting scheme as elsewhere - in particular, opening braces should not be indented. I.e.

if (foo)
{
    do_something ();

    if (bar)
    {
        do_something_else ();
    }
}

Otherwise, this looks great!

@hpjansson
Copy link
Owner

(If you prefer to just have it say (C) oupson, that's fine too - but it'd be nice to have an e-mail address there at least).

@oupson
Copy link
Contributor Author

oupson commented Jan 23, 2024

I fixed the indentation and added the copyright.

I use clang-format to format the code, is there another tool I can use to format like you ?

@hpjansson
Copy link
Owner

I fixed the indentation and added the copyright.

Thanks!

I use clang-format to format the code, is there another tool I can use to format like you ?

I don't use formatting tools, since I find them too rigid. For instance, most of the time the code should fit ~100 columns or so, but in some cases you have to go a little bit over.

You can probably get close with some carefully chosen parameters to clang-format, indent or Emacs/Vim config. I need to figure them out and write a few paragraphs about it in the HACKING doc :-) FWIW, the most important Emacs parameters are in the file variables -*- ... -*- headers.

Apart from what I mentioned above, the style in this project is pretty relaxed. Look at the existing code and do what feels right!

@hpjansson hpjansson merged commit c523a81 into hpjansson:master Jan 23, 2024
1 check passed
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