Feature/syntax highlight hover #1443

akinsho · 2018-02-06T10:01:51Z

This PR adds syntax highlighting to the hover tool tip other changes include:

Adding deep merges to the config so it supports sub object fields
Adding an adhoc highlighter to generate tokens for given lines as well as fn to generate vim highlights for a line
Added a function to get vim highlights actual color values
Added functionality to pull token colors from the json theme, user config and set defaults based on available vim highlights.
I've removed splitting up the hover info on every new line and now only render the docs and the titles differently.

Todo

Set highlight colors for various tokens or so tokens such as variable.other.object render with a color
Fix failing test due to config changes
Fix Regex quirks re .| characters
Add unit tests
resize hover container

Screenshots

Discussed below:
Fix formatting for code block and add highlighting to code blocks in tooltip

switch to string replacement method

use token names to generate classes

…ghlight-hover

split out quickInfo container and pass it its own version of the theme

bryphe · 2018-02-07T01:46:12Z

browser/src/Editor/NeovimEditor/NeovimEditor.tsx

+        }> = this._configuration.getValue("editor.tokenColors")
+
+        // FIXME: Specify Highlights to check by differentiating vim highlights as no having "."
+        const tokens = Object.keys(configurationColors).filter(c => !c.includes("."))


Could we move this logic to NeovimInstance as getHighlights? This way, we don't need a public getVimHighlights method, as I'm hoping to keep the Editor interface as agnostic of Vim as possible.

Once we have that, we could change below to:

const vimHighlights = await this._neovimInstance.getHighlights()

bryphe · 2018-02-07T01:47:09Z

browser/src/Editor/NeovimEditor/NeovimEditor.tsx

+        }> = this._configuration.getValue("editor.tokenColors")
+
+        // FIXME: Specify Highlights to check by differentiating vim highlights as no having "."
+        const tokens = Object.keys(configurationColors).filter(c => !c.includes("."))


I wonder why we need to look up editor.tokenColors here? Could we use a fixed array of `["Function", "Identifier", "String", "Character", etc], and then convert them to scopes?

I might've had the reverse in mind - we get all the Vim highlights, convert them to scopes + colors, and then use those for setting syntax highlights.

bryphe · 2018-02-07T01:48:47Z

browser/src/Services/Themes/ThemeManager.ts

@@ -319,6 +484,19 @@ export class ThemeManager {
        return this._themeLoader.getAllThemes()
    }

+    public async getVimHighlightColors(tokenColors: IVimHighlight[]) {


It seems like we're actually setting the token colors here, so we might want to rename this to setTokenColors instead.

bryphe · 2018-02-07T01:52:37Z

browser/src/Services/Themes/ThemeManager.ts

+    },
+}
+
+export interface IVimHighlight {


Ideally, we could make the ThemeManager agnostic of Vim. The highlight group is really an implementation detail - it'd be great if we could make this care about color/bold/italic/background-color, and then leave it up to the setHighlights in Buffer to rationalize highlight groups from that. It sounded like there might be challenges here though - let me know what sort of hurdles you ran into.

bryphe · 2018-02-07T01:53:38Z

browser/src/UI/components/QuickInfo.tsx

@@ -3,6 +3,79 @@ import * as os from "os"
 import * as React from "react"
 import styled, { boxShadowInset, css, fontSizeSmall, withProps } from "./common"

+const markedCss = css`


The changes for the rendering look great to me! Thanks for all your work here. I wonder if we could split this up into two changes? It seems like the synchronizing highlight groups has some challenges, but we could at least bring in the rendering with the editor.tokenColors we have today

bryphe · 2018-02-07T01:54:46Z

browser/src/UI/components/QuickInfoContainer.tsx

+    "editor.tokenColors": {
+        "meta.import": {
+            scope: "meta-import",
+            settings: "Define",


Ah ya, this is where I was hoping we could specify color/background color/bold/italic etc (and in the user's config / theme as well) instead of the vim highlight group. The vim highlight group was really just a crutch to implement.

bryphe · 2018-02-07T01:55:06Z

extensions/theme-onedark/colors/onedark.json

+        "highlight.mode.visual.foreground": "#282c34",
+        "editor.tokenColors": {
+            "variable.parameter": {
+                "color": "#d19a66"


bryphe · 2018-02-07T01:56:01Z

browser/src/Editor/BufferManager.ts

@@ -295,6 +295,9 @@ export class Buffer implements IBuffer {
        }

        const getToken = (lineContents: string, character: number): Oni.IToken => {
+            if (!lineContents || !character) {


Interesting, I wonder if this is related to an issue in my PR #1298... perhaps I'm hitting this too!

it requires setting up moving json files around as grammar loader is a dependecy and cant be used without a valid textmate json file can look into adding this functionality separately to test grammar loader

codecov · 2018-02-13T05:54:38Z

Codecov Report

Merging #1443 into master will decrease coverage by <.01%.
The diff coverage is 45.83%.

@@            Coverage Diff             @@
##           master    #1443      +/-   ##
==========================================
- Coverage   45.63%   45.62%   -0.01%     
==========================================
  Files         112      112              
  Lines        4111     4114       +3     
  Branches      589      590       +1     
==========================================
+ Hits         1876     1877       +1     
- Misses       2105     2107       +2     
  Partials      130      130

Impacted Files	Coverage Δ
browser/src/UI/components/common.ts	`92.59% <100%> (+0.28%)`	⬆️
browser/src/Services/Themes/ThemeManager.ts	`60.52% <100%> (ø)`	⬆️
...rowser/src/Services/Configuration/Configuration.ts	`73.91% <100%> (+0.28%)`	⬆️
...lugins/Api/LanguageClient/LanguageClientHelpers.ts	`50% <16.66%> (-2.78%)`	⬇️
browser/src/UI/components/QuickInfo.tsx	`30.76% <33.33%> (+0.21%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44e162d...60e6ee9. Read the comment docs.

This is because hover rendering now involves slightly more processing and without it it appearance is noticeably janky looking

remove unadvisable display table css rule

akinsho · 2018-02-17T23:01:11Z

Further update managed to fix the outstanding bug (that I'm aware of tm) which was that code blocks weren't highlighting which was actually not a commonly encountered issue as most of the markup is being rendered in p tags by marked but it was an issue with marked trimming whitespace from the text passed to the custom renderer which I've resolved. So believe this is good to go.

also remove styling as this is actually unnecessary

lack of rule causes weird at a glance issue with larger code blocks

this is not ideal as hover then has missing keys plus some functionality can be achieved without sets

marked dumps html into the out put which is less preferable and its sanitizer option breaks highlighting

bryphe · 2018-02-22T03:39:17Z

browser/test/MarkdownTests.ts

+
+describe("Markdown Conversion Functions", () => {
+    it("Scopes to string fn should correctly convert scopes to classnames", () => {
+        const className = markdown.scopesToString(["token.scope.extension"])


Thanks for the tests here! 👍

bryphe · 2018-02-22T03:41:18Z

browser/src/UI/components/QuickInfoContainer.tsx

+            )
+
+        return (
+            <TokenThemeProvider


Cool idea to separate the logic of getting the token colors from the logic of rendering

bryphe

Thanks for all the work with this, @akin909 ! I just pulled the latest and it looks awesome 💯

akinsho added 16 commits January 28, 2018 01:43

create syntax hl fn WIP

1b3623b

reduce size of QF containers

eb769bb

Merge branch 'master' into feature/syntax-highlight-hover

082c53f

[WIP] initial progress rendering highlights

e673f07

get highlights from vim pass these to theme object

a9ea49c

actually add token generator

c136f66

re-add main files ........

19dd4b6

fix weird deletion of files

efdb6bf

remove errant console.log

73403d3

remove highlighting from docs

81ed9e4

switch to string replacement method

add functionality to specify token in config and themes

bb41561

deepmerge config files rather than shallow

018cbd4

add mergeWith to ensure user theme populated

6ee8955

use token names to generate classes

re-add code block renderer

c00e4b6

add tslint console disable whilst debugging

f4b70fe

fix lint errors

aca47fe

akinsho mentioned this pull request Feb 6, 2018

Can't assign highligt to called function name in JS #1379

Open

akinsho added 2 commits February 6, 2018 19:59

add more scopes and associated classes

67c900c

Merge branch 'master' of github.com:onivim/oni into feature/syntax-hi…

d37fba2

…ghlight-hover

akinsho mentioned this pull request Feb 6, 2018

Theming: Allow specifying of token colors in json themes #1051

Open

akinsho added 2 commits February 7, 2018 00:23

fix rendering of hover text YAY!!

2f384bc

split out quickInfo container and pass it its own version of the theme

add new components file

328fa9a

bryphe reviewed Feb 7, 2018

View reviewed changes

akinsho added 4 commits February 12, 2018 14:01

add comment for the convert markdown function

140a11c

remove max-width from documentation component - not necessary

b321d8d

remove test as its value is questionable and

0cb08b8

it requires setting up moving json files around as grammar loader is a dependecy and cant be used without a valid textmate json file can look into adding this functionality separately to test grammar loader

add spurious change to rerun tests

5647afa

akinsho changed the title ~~[WIP] Feature/syntax highlight hover~~ Feature/syntax highlight hover Feb 13, 2018

akinsho added 8 commits February 13, 2018 22:19

re-add code block highlighting

e3056f4

fix mistakenly placed css

86008c3

once again move css as Again missed the target

7271b78

merge upstream use upstream changes

f89aee3

Slightly increase timeout

03b5213

This is because hover rendering now involves slightly more processing and without it it appearance is noticeably janky looking

trim white space if code block to correctly match

62c2c3e

remove unadvisable display table css rule

remove unnecessary newline char

6431e1e

update yarn lock

090eeaf

akinsho added 9 commits February 18, 2018 00:46

remove code tag as marked provides one

1b82154

also remove styling as this is actually unnecessary

Add more tokens to default tokens, improve comments

a173666

revert removal of white-space: pre for code blocks

b6bf3bf

lack of rule causes weird at a glance issue with larger code blocks

fix broken unit test

b8dabbf

Don't use sets as they remove duplicate entries

a40b818

this is not ideal as hover then has missing keys plus some functionality can be achieved without sets

fix up comment

dc29427

use dom purify instead of markeds purifier

03cd8eb

marked dumps html into the out put which is less preferable and its sanitizer option breaks highlighting

fix type annotation - since tss forced me to :(((((

8485cdd

remove codespan renderer for now

60e6ee9

akinsho mentioned this pull request Feb 20, 2018

Border in quick info tooltip #1576

Open

bryphe reviewed Feb 22, 2018

View reviewed changes

bryphe approved these changes Feb 22, 2018

View reviewed changes

akinsho merged commit b3b5b80 into onivim:master Feb 22, 2018

akinsho mentioned this pull request Mar 3, 2018

Feature/change token format #1705

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/syntax highlight hover #1443

Feature/syntax highlight hover #1443

Uh oh!

akinsho commented Feb 6, 2018 •

edited

Loading

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

bryphe Feb 7, 2018

Uh oh!

codecov bot commented Feb 13, 2018 •

edited

Loading

Uh oh!

akinsho commented Feb 17, 2018

Uh oh!

bryphe Feb 22, 2018

Uh oh!

bryphe Feb 22, 2018

Uh oh!

bryphe left a comment

Uh oh!

Uh oh!

Feature/syntax highlight hover #1443

Feature/syntax highlight hover #1443

Uh oh!

Conversation

akinsho commented Feb 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

akinsho commented Feb 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bryphe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akinsho commented Feb 6, 2018 •

edited

Loading

codecov bot commented Feb 13, 2018 •

edited

Loading