Skip to content

Nord theme#2189

Merged
jeanas merged 11 commits intopygments:masterfrom
mfurquimdev:nord-theme
Jul 15, 2022
Merged

Nord theme#2189
jeanas merged 11 commits intopygments:masterfrom
mfurquimdev:nord-theme

Conversation

@mfurquimdev
Copy link
Copy Markdown
Contributor

Hi

It's been more than one year since @yu-andy opened the MR #1799 to add the nord theme. He hasn't added the requested modifications, so I'm opening this PR as suggested by @timskovjacobsen in this PR.

The two new styles that I am adding are the original nord theme and a nord theme with a darker background (a personal preference).

This darker color is the darkest background from their website.

Both images were generated with the following command:

pygmentize -v \
           -o nord.png \
           -O hl_lines="1 3 4",style=nord,line_numbers=False,font_name=FiraCode \
           -f png \
           random_number_1_10.sh

Original nord theme
nord

Nord with darker background
nord-darker

Any suggestions or comments, just let me know and I will make the appropriate changes.

Copy link
Copy Markdown
Contributor

@jeanas jeanas left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of nits.

@jeanas
Copy link
Copy Markdown
Contributor

jeanas commented Jul 15, 2022

CI fails because this style has quite low contrast, which is a bit sad. OTOH, I'm not sure if there's much we can do about that given it's designed by other people and maintained consistent across an array of tools. I see that they increased the contrast for comments at some point, nordtheme/nord#94. Did you incorporate that change? Otherwise, I'm afraid we can't do much more than bypassing this test by editing the min_contrasts.json manually ...

@mfurquimdev
Copy link
Copy Markdown
Contributor Author

I didn't know about this change they've made. I just updated it to include the nord3 + 10%. If it's still too low contrast, I can increase it further up to 20%.

@jeanas
Copy link
Copy Markdown
Contributor

jeanas commented Jul 15, 2022

Also, the file header check fails, you need to do

diff --git a/pygments/styles/nord.py b/pygments/styles/nord.py
index 94c7f23a..15b6d5cb 100644
--- a/pygments/styles/nord.py
+++ b/pygments/styles/nord.py
@@ -1,12 +1,12 @@
 """
     pygments.styles.nord
-    ~~~~~~~~~~~~~~~~~~~~~~~
+    ~~~~~~~~~~~~~~~~~~~~
 
     pygments version of the "nord" theme by Arctic Ice Studio
     https://www.nordtheme.com/
 
-    :copyright: Copyright 2006-2021 by the Pygments team, see AUTHORS.
-    :license: BSD-3 Clause License. See LICENSE for details
+    :copyright: Copyright 2006-2022 by the Pygments team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
 """
 

@jeanas
Copy link
Copy Markdown
Contributor

jeanas commented Jul 15, 2022

As they apparently more or less standardized on 10%, I think it's OK to stay with that. This style is not going to be a high-contrast one in any case. That's fine; if you use Pygments just for yourself or for a specific set of people you know, and nobody visually impaired is expected to view the output, a low-contrast style is OK. It's also perfectly OK to provide two versions of the same content, one of which is adapted for low vision. IMHO, we just need to state clearly that the contrast is low, which the style gallery at https://pygments.org/styles/ does.

So I suggest you do:

diff --git a/tests/contrast/min_contrasts.json b/tests/contrast/min_contrasts.json
index ee804304..c8002653 100644
--- a/tests/contrast/min_contrasts.json
+++ b/tests/contrast/min_contrasts.json
@@ -42,5 +42,7 @@
   "gruvbox-light": 3.2,
   "dracula": 1.4,
   "one-dark": 3.7,
-  "lilypond": 2.3
+  "lilypond": 2.3,
+  "nord": 2.4,
+  "nord-darker": 2.8
 }
\ No newline at end of file

@mfurquimdev
Copy link
Copy Markdown
Contributor Author

Just out of curiosity, what are those numbers in tests/contrast/min_contrasts.json?

@jeanas
Copy link
Copy Markdown
Contributor

jeanas commented Jul 15, 2022

It maps style names to min contrast. The contrasts are measured according to the WCAG indicator. The min contrast is just the minimum of the contrasts for each of the tokens in a style.

I got those figures by running python scripts/update_contrasts.py after having commented out the line that tests they're high enough.

@jeanas jeanas merged commit caff261 into pygments:master Jul 15, 2022
@Anteru Anteru added this to the 2.13.0 milestone Aug 14, 2022
@Anteru Anteru added the A-theming area: changes to themes label Aug 14, 2022
This was referenced Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-theming area: changes to themes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants