Vscode: Add comment snippet variable

[gandalfsaxe]

I would be really useful to have a special syntax/keyword/variable for global snippets such that you could insert a comment character, whatever it looks like for the current language. I.e. for python #, for LaTeX % etc.

[jrieken]

@GandalfSaxe Can you provide more detail or a sample? How does the prefix not help here?

[V-ed]

Interesting - I can already imagine a snippet that adds a TODO with my name in comment based on the language that I'm currently using

snippet :

"Insert TODO": {
    "prefix": "todome",
    "body": [
        "TODO : @V-ed"
    ],
    "description": "Inserts a TODO with my name",
    "isComment": true        // Example, I have no idea how this would be implemented with TextMate
}

---

java

// TODO : @V-ed

html

<!-- TODO : @V-ed -->

css

/* TODO : @V-ed */

---

Now, what about auto comments inside the snippet... Something along those lines?

"Insert TODO": {
    "prefix": "todome",
    "body": [
        "var e = null; ${c:TODO : @V-ed}"
    ],
    "description": "Inserts a TODO with my name"
}

[jrieken]

Ok, understood. Should this prefer a line comment over a block comment?

[V-ed]

Depends on how it's implemented, but I suggest using either one depending on the casing of the comment variable

Line comment :

"Insert TODO": {
    "prefix": "todome",
    "body": [
        "var e = null; ${c:TODO : @V-ed}"
    ],
    "description": "Inserts a TODO with my name"
}

java : var e = null; //TODO : @V-ed

-- ~ --

Block comment :

"Insert TODO": {
    "prefix": "todome",
    "body": [
        "var e = null; ${C:TODO : @V-ed}"
    ],
    "description": "Inserts a TODO with my name"
}

java : var e = null; /*TODO : @V-ed*/

[gandalfsaxe]

I like the idea of line/block comment depending on casing.

@V-ed How would you then make multiple lines enclosed as a block comment? As far as I can see, both your snippet examples are one-liners.

[V-ed]

Well, given how weird snippets are handled in VSCode (_*cough cough* #15140_), I guess something like this would do the job :

"Insert null var": {
    "prefix": "varnull",
    "body": [
        "${c:",
        " Null variable initialisation",
        "}",
        "var e = null;",
    ],
    "description": "Inserts a null variable with doc"
}

"c" -> line comment results for languages

java

//
// Null variable initialisation
//
var e = null;

html (does not make sense but the idea is there)

<!--
 Null variable initialisation
-->
var e = null;

css (does not make sense, but the idea is there)

/*
 Null variable initialisation
*/
var e = null;

---

"Insert null var": {
    "prefix": "varnull",
    "body": [
        "${C:",
        " Null variable initialisation",
        "}",
        "var e = null;",
    ],
    "description": "Inserts a null variable with doc"
}

"C" -> block comment results for languages

java

/*
 Null variable initialisation
*/
var e = null;

html (does not make sense but the idea is there)

<!--
 Null variable initialisation
-->
var e = null;

css (does not make sense, but the idea is there)

/*
 Null variable initialisation
*/
var e = null;

---

Now comes an interesting subject to talk about : how should it handle block comment's format? In my multiline block comment of java, there is only a /* [...] */ on the first line and at the last line, which is a valid block comment, but it is very common to have, for example, something like this :

/*
* This is a block comment
*/

which we may not have in other languages.

How should that be handled? should it be per language server (a kind of "snippet comment format" setting somewhere)? should there be a way for the user to choose which format his snippet should apply (so a user setting or a "snippet setting", kind of like the "description" data in current snippets)? Should it be only a default style, so start a block comment, don't manipulate any other string and end with the end block comment string?

All of those questions makes it hard for me to imagine how such a feature would be implemented (also, is this even following Textmate? Is following TextMate even a requirement?). One's thing for sure, this kind of per-language-snippet-settings is interesting and could potentially expand to even more than comments only (creating a boolean variable in a particular language eh?) - we've gotta agree on at least a way to make it consistent for snippet creators, that'd be a start.

[V-ed]

To further up my knowledge about how snippets work (and confirm the example I've given in my previous comment), I've created the following snippet to test if the selection does indeed take multiple lines (which in turn would confirm how the comments should work in multiline) :

"testSnippet": {
    "prefix": "sniptest",
    "body": [
        "This is a\r",
        "\\tmulti ${1:line\r",
        "text}"
    ],
    "description": "no"
}

and it indeed does do what I expected :

This is a
    multi <line
text>

(text between <> is selected)

This kinda supports my idea of doing it like what's underneat for multiline snippets :

"Insert null var": {
    "prefix": "varnull",
    "body": [
        "${c:",
        " Null variable initialisation",
        "}",
        "var e = null;",
    ],
    "description": "Inserts a null variable with doc"
}

[gandalfsaxe]

Looks good to me. Hope someone could take time to implement at some point 🤞 🙂

[hhu94]

@V-ed Seeing as VSCode already has actions for toggling line and block comments, I'd imagine snippets could leverage that functionality, reducing the amount of design decisions that need to be made.

[V-ed]

@hhu94 Seems like a good idea, but then could we be able to get both block comments and line comments in a single snippet?

Given this snippet result example :

/*
This is a paragraph.
It might explain something that is repetitive and used in alot of projects.
We may never know...
*/
var test = null; // TODO : @team, verify QA

How could you differenciate the comments type in this snippet without some type of handling in the snippet itself (aka with the toggle line and block comments feature)?

[hhu94]

@V-ed I think something like this could work right?

"Insert null var": {
    "prefix": "varnull",
    "body": [
        "${C:",
        "This is a paragraph.",
        "It might explain something that is repetitive and used in alot of projects.",
        "We may never know...",
        "}",
        "var e = null; ${c:TODO : @team, verify QA}",
    ],
    "description": "Inserts a null variable with doc"
}

Apply the toggling of line / block comments in the specific ranges specified by the snippet.

[V-ed]

@hhu94 Oh, I think I misunderstood you at first then, you mean to add to my idea of using cased c and C, but when toggling line / block comments using VSCode's already existing command, the command would be applied to all of the ranges in the snippet?

(using the snippet you sent as example) :
at first you would get

/*
This is a paragraph.
It might explain something that is repetitive and used in alot of projects.
We may never know...
*/
var test = null; // TODO : @team, verify QA

and then, after using the toggle comments action from vscode, it would turn into this :

//
//This is a paragraph.
//It might explain something that is repetitive and used in alot of projects.
//We may never know...
//
var test = null; /* TODO : @team, verify QA */

Am I understanding correctly?

[hhu94]

@V-ed I think there might have been some confusion about what I meant by toggling. What I meant by toggling line / block comments is these two actions that vscode already supports.

My idea was that we could use this functionality to implement how ${C:...} and ${c:...} would turn normal text into comments. And since these actions are language aware, you wouldn't need to worry about design decisions like your concern about multiline block comments in Java.

[V-ed]

@hhu94 Oh, I was completely off track with what I had understood wasn't I :) you were even very clear in your first comment, I saw it from the wrong eye and confused myself more than anything...

Yes, it would be interesting to leverage that functionality like you say, and pretty much all of my questions from https://github.com/Microsoft/vscode/issues/55946#issuecomment-411176961 would be answered as it would use a way that is already established - that would be neat. Why reinvent the wheel? :)

[vidalon]

Hey, I want to make my first contribution to open source and to this project by working on this issue. Let me know if you have any pointers beyond what has already been discussed here.

[karanisverma]

@vidalon are you working on it?
if no one is working on this issue, I would like to pick this up!
@GandalfSaxe @V-ed please let me know 🙂

[gandalfsaxe]

I’m not working on it :)

[vidalon]

Hey, I have not made much progress. Feel free to take it.

On Tue, Oct 23, 2018 at 10:31 AM Gandalf Saxe [e-mail]
wrote:

I’m not working on it :)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/vscode/issues/55946#issuecomment-432292089,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAXWauyuYJhFys-g3G4LHVGyRFhmoFm6ks5unzZOgaJpZM4VyOZ3
.

[karanisverma]

Cool 👍 🙂

[karanisverma]

Depends on how it's implemented, but I suggest using either one depending on the casing of the comment variable

Line comment :

"Insert TODO": {
    "prefix": "todome",
    "body": [
        "var e = null; ${c:TODO : @V-ed}"
    ],
    "description": "Inserts a TODO with my name"
}

java : var e = null; //TODO : @V-ed

-- ~ --

Block comment :

"Insert TODO": {
    "prefix": "todome",
    "body": [
        "var e = null; ${C:TODO : @V-ed}"
    ],
    "description": "Inserts a TODO with my name"
}

java : var e = null; /*TODO : @V-ed*/

@jrieken should i proceed with this approach or is there any approach i should be looking for?
meanwhile I am trying to understand how commenting works currently.

[jrieken]

We should stick to TextMate snippets and how variables usually work. Eg. have a few new variables like $LINE_COMMENT which resolves to // or # etc and $BLOCK_COMMENT_START, $BLOCK_COMMEND_END. The actual values can be read from our services so that it works language agnostic. A sample would be

"body": [
     "var e = null; ${LINE_COMMENT:TODO : @V-ed}"
],

and

"body": [
  "$BLOCK_COMMENT_START"
  "Copyright header"
  "$BLOCK_COMMENT_END"
],

[PedroMiguelSilva]

I've been trying to tackle this issue and mostly getting comfortable with the project layout. I noticed some languages have a 'snippet' json under /extensions/{languageName}/snippets/{languageName}.json. However, besides javascript and few others, none of them have the most used snippets I was expecting to find except for the tag #region and #endregion.

The bad approach would be having a snippet in each language json but that doesn't sound reasonable for a project like this.
How can i set up these 'global variables' in order to make the snippets as versatile as possible?
@jrieken

[karanisverma]

We should stick to TextMate snippets and how variables usually work. Eg. have a few new variables like $LINE_COMMENT which resolves to // or # etc and $BLOCK_COMMENT_START, $BLOCK_COMMEND_END. The actual values can be read from our services so that it works language agnostic. A sample would be

"body": [
     "var e = null; ${LINE_COMMENT:TODO : @V-ed}"
],

and

"body": [
  "$BLOCK_COMMENT_START"
  "Copyright header"
  "$BLOCK_COMMENT_END"
],

Hi @jrieken I went through the code base and I can see in file /src/vs/editor/contrib/snippet/snippetVariables.ts has variable and resolvers for the same. I am wondering should I create new resolver for comments variable as it doesn't match with any of the following resolvers:

CompositeSnippetVariableResolver
SelectionBasedVariableResolver
ModelBasedVariableResolver
ClipboardBasedVariableResolver
TimeBasedVariableResolver

I am thinking of naming new resolver are CommentBasedVariableResolver
I have made changes snippetVariables.ts file
I am sharing possible implementation which should work.

export class CommentBasedVariableResolver implements VariableResolver {
    resolve(variable: Variable): string | undefined {
        const { name } = variable;
        // const languageId = <NOT SURE HOW TO GET LANGUAGE ID HERE>;
        const comments = LanguageConfigurationRegistry.getComments(languageId);
        if (name == 'LINE_COMMENT') {
            return comments.lineCommentToken
        } else if (name == 'BLOCK_COMMENT_START') {
            return comments.blockCommentStartToken
        } else if (name == 'BLOCK_COMMENT_END') {
            return comments.blockCommentEndToken
        }
        return undefined
    }
}

I am not sure how do i get LanguageId here.

[karanisverma]

@jrieken After spending some more time I have come to this solution for getting language id:
const language = this._model.getLanguageIdentifier()

export class CommentBasedVariableResolver implements VariableResolver {
    constructor(
        private readonly _model: ITextModel
    ) {
        //
    }
    resolve(variable: Variable): string | undefined {
        const { name } = variable;
        const language = this._model.getLanguageIdentifier()
        const comments = LanguageConfigurationRegistry.getComments(language.id);
        if (name == 'LINE_COMMENT') {
            return comments.lineCommentToken
        } else if (name == 'BLOCK_COMMENT_START') {
            return comments.blockCommentStartToken
        } else if (name == 'BLOCK_COMMENT_END') {
            return comments.blockCommentEndToken
        }
        return undefined
    }
}

Please let me know if it look good. I will make this changes and create a PR for the same.

[jrieken]

@karanisverma You are on the right track!

[karanisverma]

@karanisverma You are on the right track!

Thanks @jrieken !
I have tested above changes, It seems to work as intendant(resolving to correct comment syntax with different language) however except with ${name:default} where variable is getting resolve as an empty string instead of it's value. I will be creating PR, It would be great if you can point out what I am missing here.

    "Insert TODO": {
        "prefix": "commentBlock",
        "body": [
            "${BLOCK_COMMENT_START}"
            "Copyright header"
            "${BLOCK_COMMENT_END}"
        ],
        "description": "Inserts a TODO with my name"
    },
    "Insert TODO2": {
        "prefix": "commentLine",
        "body": [
            "var e = null; ${LINE_COMMENT:TODO : @V-ed}"
        ],
        "description": "Inserts a TODO with my name"
    }

commentLine is getting resolved as in .js file to
var e = null; //
instead of
var e = null; // TODO: @V-ed

commentBlock snippet seems to work fine.
PR: https://github.com/Microsoft/vscode/pull/63572/files

[jrieken]

where variable is getting resolve as an empty string instead of it's value.

use undefined instead of the empty string

[karanisverma]

where variable is getting resolve as an empty string instead of it's value.

use undefined instead of the empty string

I did to get what you just said, not sure where i have to use undefined
In CommentBasedVariableResolver I am returning undefined if it doesn't match with any of the variable names. ( https://github.com/Microsoft/vscode/pull/63572/files#diff-e192a0aee0e936a742ea5a87bf83aa30R204 )

To elaborate more here are two snippet

    "Insert TODO": {
        "prefix": "commentLine",
        "body": [
            "var e = null; ${LINE_COMMENT:TODO @V-ed}"
        ],
        "description": "Inserts a TODO with my name"
    },
    "Insert TODO2": {
        "prefix": "commentLine2",
        "body": [
            "var e = null; ${LINE_COMMENT} TODO @V-ed"
        ],
        "description": "Inserts a TODO with my name"
    }

and their output respectively(in a js file)

var e = null; //
var e = null; // TODO @V-ed

My understanding is if "var e = null; ${LINE_COMMENT:TODO @V-ed}" should return output like
var e = null; // TODO @V-ed. however it generate output like var e = null; //.

It would be great if you can review and PR and suggest the changes there 🙂

[jrieken]

My understanding is if "var e = null; ${LINE_COMMENT:TODO @V-ed}" should return output like
var e = null; // TODO @V-ed. however I am missing something here.

Yeah, that's correct. The empty string however doesn't trigger the default only undefined does. So, return undefined when the variable isn't matching but also when receiving the empty string from the language registry

[jrieken]

👏 @karanisverma

[karanisverma]

Yay! Thanks a lot @jrieken 🙌🏼
Looking forward for contribute more 😃

[gandalfsaxe]

Very cool 😀👍

[jrieken]

Verifier: Make sure that you can now use these snippet variables and that they correspond to the language: BLOCK_COMMENT_START, BLOCK_COMMENT_END, LINE_COMMENT