dhaumann requested changes to this revision.
dhaumann added a comment.
This revision now requires changes to proceed.


  Looks almost good, but I think another iteration would be nice.
  
  I assume we cannot relicense to MIT?

INLINE COMMENTS

> powershell.xml:5
> +  version="1"
> +  kateversion="3.4"
> +  extensions="*.ps1;*.ps1m;*.ps1d"

If I remember correctly, the default style dsBuiltIn was added with the first 
release of KDE Frameworks 5, see all bold entries here:
https://kate-editor.org/2014/03/07/kate-part-kf5-new-default-styles-for-better-color-schemes/

So kateversion="5.0" is required here.

> powershell.xml:918
> +      <itemData name="String"       defStyleNum="dsString"/>
> +      <itemData name="HereString"       defStyleNum="dsString"/>
> +      <itemData name="Comment"      defStyleNum="dsComment"/>

Since we require kateversion 5.0 anyways, we could also use dsVerbatimString 
instead of dsString.

> powershell.xml:922
> +      <itemData name="Symbol"       defStyleNum="dsNormal"/>
> +      <itemData name="Variable" defStyleNum="dsKeyword" color="#5555FF" 
> selColor="#ffffff" bold="0" italic="0" spellChecking="false" />
> +      <itemData name="Special Variable" defStyleNum="dsKeyword" 
> color="#5555FF" selColor="#ffffff" bold="1" italic="0" spellChecking="false" 
> />

Suggestion: use dsVariable for variables, and remove all hardcoded colors and 
bold and italic.

> powershell.xml:923
> +      <itemData name="Variable" defStyleNum="dsKeyword" color="#5555FF" 
> selColor="#ffffff" bold="0" italic="0" spellChecking="false" />
> +      <itemData name="Special Variable" defStyleNum="dsKeyword" 
> color="#5555FF" selColor="#ffffff" bold="1" italic="0" spellChecking="false" 
> />
> +    </itemDatas>

dsConstant would be better.

REPOSITORY
  R216 Syntax Highlighting

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

To: vkrause, dhaumann, vonreth
Cc: #frameworks

Reply via email to