andreasgr added a comment.

  In D18475#400238 <https://phabricator.kde.org/D18475#400238>, @nibags wrote:
  
  > Hi!!
  >  Please, also add a test file in: "autotests/input/". To generate the files 
in "folding", "html" and "reference" use `make test` and 
`autotests/update-reference-data.sh`.
  >
  > I am not a reviewer, but I want to add some recommendations in relation to 
the itemData's:
  >
  > - On lines 467, 476 and 485 you use harded colors for the fonts. Ideally, 
don't use them, but use the default styles, since a better visualization is 
achieved in the different schemes: 
https://kate-editor.org/2014/03/07/kate-part-kf5-new-default-styles-for-better-color-schemes/
  > - On lines 469 & 471 you use background color, and on lines 466-467, 
473-474, 479-481 and 490 you use background color, but not font color. Try not 
to use background color if it's not necessary,  to get a better visualization 
in the different schemes. However, if you need background color, it also 
specifies the font color, to avoid readability problems in some color schemes 
(for example, the background color `&monospaced_bg;` with `dsNormal` is not 
readable in dark schemes).
  
  
  Hi @nibags,
  
  many thanks for your feedback.
  
  I will see what I can do about the test file.
  
  Regarding colours, this was mostly done to make the highlighting look similar 
to the default output created by Asciidoctor.
  Your point about different schemes is a very valid one, though. I need to 
reconsider the style choices.
  
  Cheers

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D18475

To: andreasgr, #framework_syntax_highlighting
Cc: nibags, kwrite-devel, kde-frameworks-devel, #framework_syntax_highlighting, 
bmortimer, hase, michaelh, genethomas, ngraham, bruns, demsking, cullmann, 
vkrause, sars, dhaumann

Reply via email to