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

emojione: add fontconfig module #30936

Closed
wants to merge 1 commit into from

Conversation

bricewge
Copy link
Contributor

Motivation for this change

It add a new NixOS option to add emojjione's config to fontconfig. When enabled emoji characters are render with only one font –emojione– and not several as it is currently. It can be seen here: http://getemoji.com/

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@vyp
Copy link
Member

vyp commented Oct 29, 2017

Have you tested this? I don't think it will work because NixOS' version of cairo doesn't support colour glyphs yet, though maybe I'm wrong.

@bricewge
Copy link
Contributor Author

Yes, I have being using it for some months now.
Displaying colored emoji already work in NixOS, the issue was the absence of consistency in the fonts used to displaying emoji. With the option enabled all the emoji are displayed with the emojione font.

@vyp
Copy link
Member

vyp commented Oct 30, 2017

Does it work only in the browser or everywhere?

@bricewge
Copy link
Contributor Author

bricewge commented Nov 5, 2017

You're right it only work in the browser, other software support only black and white emojii. For the moment I view emojii only in the browser so I didn't checked in other apps.

type = types.bool;
default = false;
description = ''
Enable emojione settings.
Copy link
Member

Choose a reason for hiding this comment

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

Could we change this to something more descriptive, such as Display all emoji with the emojione font.?


config = mkIf (config.fonts.fontconfig.enable && cfg.enable) {

fonts.fontconfig.confPackages = [ confPkg ];
Copy link
Member

Choose a reason for hiding this comment

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

Besides installing the Fontconfig files, I think this module should install the emojione font. Otherwise, users must make two settings in their configuration.nix.

@lukateras
Copy link
Member

I don't think this is the right approach. I think we should add fonts.fontconfig.overrides = [ "Noto Color Emoji" ]; that would generate the following config (with priority higher than 73):

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE fontconfig SYSTEM "fonts.dtd">
<fontconfig>
  <match target="pattern">
    <edit name="family" mode="prepend">
      <string>Noto Color Emoji</string>
    </edit>
  </match>
</fontconfig>

@lukateras
Copy link
Member

Also, that might not be needed anymore in the latest fontconfig.

@bricewge
Copy link
Contributor Author

Thanks for the comment @yegortimoshenko, that was what I was looking for when I wrote this push request. This PR works but seems dirty. I'll test your way of having emoji on NixOS. However I think there should be a option to enable emoji or at least a clear documentation on how to enable it.

@lukateras
Copy link
Member

lukateras commented Nov 29, 2017

@bricewge I think that was fixed in https://bugs.freedesktop.org/show_bug.cgi?id=94551, i.e. it should automatically discover proper color emoji set w/o falling back to DejaVu.

@bricewge bricewge closed this Oct 25, 2018
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.

None yet

4 participants