microsoft / monaco-editor

A browser based code editor
https://microsoft.github.io/monaco-editor/
MIT License
40.44k stars 3.6k forks source link

Semantic highlighting does not appear to work (due to theming) #1833

Open rcjsuen opened 4 years ago

rcjsuen commented 4 years ago
  1. I expect the first four characters to be highlighted but nothing happens.
  2. There is no documentation really for any of the functions but releaseDocumentSemanticTokens in particular is not clear to me what I should be doing there.

    
    monaco.languages.register({
    id: 'new'
    });
    monaco.languages.registerDocumentSemanticTokensProvider('new', {
    getLegend: () => {
        return {
            tokenModifiers: [],
            tokenTypes: [
                "keyword"
            ]
        }
    },
    
    provideDocumentSemanticTokens: () => {
        return {
            data: [
                0, 0, 4, 0, 0
            ]
        }
    },
    
    releaseDocumentSemanticTokens: () => {
    }
    });

monaco.editor.create(document.getElementById("container"), { value: "12345678", language: "new" });

alexdima commented 4 years ago

The standalone APIs are all hooked up, but I believe there is no theming implemented for the standalone editor. So those semantic tokens get looked up, no theming returns, so the semantic tokens are dropped.

https://github.com/microsoft/vscode/blob/37f2127173cf8d3185ec0ea27a7643fdcddbb07b/src/vs/editor/standalone/browser/standaloneThemeServiceImpl.ts#L134-L136

@aeschli getTokenStyleMetadata would need to be implemented, currently return undefined leads to the tokens getting dropped.

rcjsuen commented 4 years ago

@alexdima @aeschli Is it possible for me to workaround this (by calling internal APIs or whatever) or is it all too tightly coupled that I should just wait for a bug fix release?

Thank you for your information.

alexdima commented 4 years ago

@rcjsuen You could try to do this by providing a custom theme service, (with the first created editor) e.g.

monaco.editor.create(element, opts, { themeService: yourCustomImplementation });

You would then need to implement IThemeService:

export interface IThemeService {
    _serviceBrand: undefined;

    getTheme(): ITheme;

    readonly onThemeChange: Event<ITheme>;

    getIconTheme(): IIconTheme;

    readonly onIconThemeChange: Event<IIconTheme>;

}
rcjsuen commented 4 years ago

@alexdima I tried a simpler approach but nothing seems to be working. Since provideDocumentSemanticTokens is not being printed to the console I am of the belief that Monaco is not even polling the provider for tokens. Nothing is being printed from my replaced getTokenStyleMetadata function either.

monaco.languages.register({
    id: 'new'
});
monaco.languages.registerDocumentSemanticTokensProvider('new', {
    getLegend: () => {
        return {
            tokenModifiers: [],
            tokenTypes: [
                "keyword"
            ]
        }
    },

    provideDocumentSemanticTokens: () => {
        console.log("provideDocumentSemanticTokens");
        debugger;
        return {
            data: [
                0, 0, 4, 0, 0
            ]
        }
    },

    releaseDocumentSemanticTokens: () => {
    }
});

const editor = monaco.editor.create(document.getElementById("container"), {
    value: "12345678",
    language: "new"
});
const ts = editor._themeService;
const t = ts._theme;
t.getTokenStyleMetadata = (type, modifiers) => {
    console.log(type);
    console.log(modifiers);
    return undefined;
};
alexdima commented 4 years ago

@rcjsuen This is indeed a mess, and sorry about that. Semantic highlighting has shipped turned off by default. Due to it being a global setting (not an editor instance setting), it was also overlooked in monaco.d.ts typings, where it would need to be added manually...

The way to enable it is to use: 'semanticHighlighting.enabled': true as an option, i.e.

const editor = monaco.editor.create(document.getElementById("container"), {
    value: "12345678",
    language: "new",
    'semanticHighlighting.enabled': true
});

The weird dot keyname must be used due to some more trouble with object-like global settings which are not picked up, so using semanticHighlighting: { enabled: true} does not work ...

A full working sample:

monaco.languages.register({
    id: 'new'
});
monaco.languages.registerDocumentSemanticTokensProvider('new', {
    getLegend: () => {
        return {
            tokenModifiers: [],
            tokenTypes: [
                "keyword"
            ]
        }
    },

    provideDocumentSemanticTokens: () => {
        return {
            data: [
                0, 0, 4, 0, 0
            ]
        }
    },

    releaseDocumentSemanticTokens: () => {
    }
});

const editor = monaco.editor.create(document.getElementById("container"), {
    value: "12345678",
    language: "new",
    'semanticHighlighting.enabled': true
});
const t = editor._themeService._theme;
t.getTokenStyleMetadata = (type, modifiers) => {
    if (type === 'keyword') {
        return {
            foreground: 5, // color id 5
            bold: true,
            underline: true,
            italic: true
        };
    }
};

image

rcjsuen commented 4 years ago

@alexdima Thank you very much for your fully working sample! I adapted your model to my code and found that the use of a model seems to break the semantic highlighting. :( Please see the attached playground sample. If you toggle works variable between true and false and run it then you can see the differences in behaviour. 🤔

monaco.languages.register({
    id: 'new'
});
monaco.languages.registerDocumentSemanticTokensProvider('new', {
    getLegend: () => {
        return {
            tokenModifiers: [],
            tokenTypes: [
                "keyword"
            ]
        }
    },

    provideDocumentSemanticTokens: () => {
        return {
            data: [
                0, 0, 4, 0, 0
            ]
        }
    },

    releaseDocumentSemanticTokens: () => {
    }
});

let works = false;
let options = {
    'semanticHighlighting.enabled': true
};
if (works) {
    options["value"] = "12345678";
    options["language"] = "new";
} else {
    const model = monaco.editor.createModel("12345678", "new");
    options["model"] = model;
}
editor = monaco.editor.create(document.getElementById("container"), options);

const t = editor._themeService._theme;
t.getTokenStyleMetadata = (type, modifiers) => {
    if (type === 'keyword') {
        return {
            foreground: 13, // color id 13
            bold: false,
            underline: false,
            italic: false
        };
    }
};
rcjsuen commented 4 years ago

Other than the bug above, I was able to get something off the ground so it's now published online.

#escape=\
FROM node:alpine AS build
ARG aaa
RUN $aaa
#comment
HEALTHCHECK --interval=30s --timeout=30s CMD echo "var"
COPY lib /docker-langserver/lib
COPY bin /docker-langserver/bin
COPY package.json /docker-langserver/package.json
WORKDIR /docker-langserver/
RUN npm install --production && \
    chmod +x /docker-langserver/bin/docker-langserver
ENTRYPOINT [ "/docker-langserver/bin/docker-langserver" ]
image
alexdima commented 4 years ago

@rcjsuen Yes, there is a problem again rooted in the default value of semanticHighlighting.enabled being false in the shipped version. The model gets created at a time when the setting is false, and then the editor gets created with semanticHighlighting.enabled, which changes the setting in the configuration service, but the simple configuration service forgets to emit an event, so the semantic highlight feature fails to react and begin semantic coloring for the file.

rcjsuen commented 4 years ago

@alexdima Makes sense to me. Thank you for your information and all your help!

alexdima commented 4 years ago

@rcjsuen I've pushed a fix for the case you hit, so now the simple configuration service also emits a change event correctly.

rcjsuen commented 4 years ago

@aeschli @alexdima How can I help move this forward? Though I realize it's probably too late for the next release given the timeline noted in microsoft/vscode#90371.

rcjsuen commented 4 years ago

@aeschli @alexdima Now that microsoft/vscode#86415 has been finalized, can this be prioritized for April?

schickling commented 3 years ago

Is there any update on this topic @alexdima? (Related question on Stackoverflow.)

alexdima commented 3 years ago

Yes. AFAIK this works somewhat by using theme rules, but the theme rules cannot select semantic token modifiers, they can only target semantic token types and partially modifiers. So this issue is not 100% done.

There is an example of theming working with the semantic token types at https://microsoft.github.io/monaco-editor/playground.html#extending-language-services-semantic-tokens-provider-example

schickling commented 3 years ago

I see. Thanks for the update @alexdima!

Is there some way to (easily) make the respective registerDocumentSemanticTokensProvider calls for Monaco to treat things the same way as in VSC?

So this issue is not 100% done.

Are there some plans to address this?

calvinusesyourcode commented 1 month ago

Would be nice!