New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed:generator method of class autocomplete causes star(*) to be add… #51557

Merged
merged 3 commits into from Sep 11, 2018

Conversation

Projects
None yet
7 participants
@limerickgds

limerickgds commented Jun 10, 2018

i add a onEnterRule rule that will only execute if the text above the this line matches /** or *.
but i don't think this is a good solution.
Fixes #43469
cc: @joaomoreno

@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Jun 10, 2018

CLA assistant check
All CLA requirements met.

msftclas commented Jun 10, 2018

CLA assistant check
All CLA requirements met.

@limerickgds

This comment has been minimized.

Show comment
Hide comment
@limerickgds

limerickgds Jul 6, 2018

@misolori do you need anything from me to merge this PR?

limerickgds commented Jul 6, 2018

@misolori do you need anything from me to merge this PR?

@misolori misolori removed their assignment Jul 6, 2018

@misolori

This comment has been minimized.

Show comment
Hide comment
@misolori

misolori Jul 6, 2018

Member

@limerickgds I'll let @aeschli handle this PR

Member

misolori commented Jul 6, 2018

@limerickgds I'll let @aeschli handle this PR

@mjbvz mjbvz self-assigned this Jul 19, 2018

@mjbvz

This comment has been minimized.

Show comment
Hide comment
@mjbvz

mjbvz Jul 19, 2018

Contributor

@alexandrudima / @rebornix Can you please take a look at this

Contributor

mjbvz commented Jul 19, 2018

@alexandrudima / @rebornix Can you please take a look at this

@limerickgds

This comment has been minimized.

Show comment
Hide comment
@limerickgds

limerickgds Jul 25, 2018

@mjbvz @misolori @aeschli How long do I have to wait?

limerickgds commented Jul 25, 2018

@mjbvz @misolori @aeschli How long do I have to wait?

@aeschli

This comment has been minimized.

Show comment
Hide comment
@aeschli

aeschli Jul 26, 2018

Contributor

@alexandrudima and @rebornix have the final word here, but looking at the previous line looks fishy to me. For other language configurations we can already express where the rule applies:

	"autoClosingPairs": [
		{ "open": "/*", "close": "*/", "notIn": ["string"] },
		{ "open": "\"", "close": "\"", "notIn": ["string", "comment"] },
	]

See

export interface IAutoClosingPairConditional extends IAutoClosingPair {
.

Ideally, we can add the same for OnEnterRule

Contributor

aeschli commented Jul 26, 2018

@alexandrudima and @rebornix have the final word here, but looking at the previous line looks fishy to me. For other language configurations we can already express where the rule applies:

	"autoClosingPairs": [
		{ "open": "/*", "close": "*/", "notIn": ["string"] },
		{ "open": "\"", "close": "\"", "notIn": ["string", "comment"] },
	]

See

export interface IAutoClosingPairConditional extends IAutoClosingPair {
.

Ideally, we can add the same for OnEnterRule

@limerickgds

This comment has been minimized.

Show comment
Hide comment
@limerickgds

limerickgds Sep 8, 2018

@aeschli do you need i change this PR ?

limerickgds commented Sep 8, 2018

@aeschli do you need i change this PR ?

@alexandrudima

This comment has been minimized.

Show comment
Hide comment
@alexandrudima

alexandrudima Sep 11, 2018

Member

I personally kind of like it that the implemention of onEnterRules is kept tokenization agnostic and I think the new rule (oneLineAboveText) is in the same spirit with my original implemention (i.e. similar to beforeText and afterText).

@limerickgds Thank you !

Member

alexandrudima commented Sep 11, 2018

I personally kind of like it that the implemention of onEnterRules is kept tokenization agnostic and I think the new rule (oneLineAboveText) is in the same spirit with my original implemention (i.e. similar to beforeText and afterText).

@limerickgds Thank you !

@alexandrudima alexandrudima merged commit 1026e5f into Microsoft:master Sep 11, 2018

1 of 2 checks passed

VS Code queued
Details
license/cla All CLA requirements met.
Details

@alexandrudima alexandrudima added this to the September 2018 milestone Sep 11, 2018

Show resolved Hide resolved src/vs/vscode.d.ts
@jrieken

This comment has been minimized.

Show comment
Hide comment
@jrieken

jrieken Sep 11, 2018

Member

creates #58440 as tracking item

Member

jrieken commented Sep 11, 2018

creates #58440 as tracking item

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment