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

Add multiline description support in user snippets #66159

Merged
merged 1 commit into from Jan 10, 2019

Conversation

@jamesgeorge007
Copy link
Contributor

jamesgeorge007 commented Jan 7, 2019

Closes #66036

Present scenario

Inorder to show a lot of description, it is required must write a huge string which includes many escape characters the one given below.

"toctree": {
	"prefix": ".. toctree::",
	"body": [
		".. toctree::",
		"$0"
	],
	"description": "A toctree item, have these options:\n\t:maxdepth: para\n\t:caption: para
\n\t:numbered:\n\t:titlesonly:\n\t:glob:\n\t:reversed:\n\t:hidden:\n\t:includehidden:",
	"scope": "text.restructuredtext"
},

Proposed solution

"toctree": {
	"prefix": ".. toctree::",
	"body": [
		".. toctree::",
		"$0"
	],
	"description": [
		"A toctree item, have these options:",
		":maxdepth: para",
		":caption: para",
		":numbered:",
		":titlesonly:",
		":glob:",
		":reversed:",
		":hidden:",
		":includehidden:"
	],
	"scope": "text.restructuredtext"
},

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch 2 times, most recently from 963103e to 6bc73d8 Jan 8, 2019

@jrieken

This comment has been minimized.

Copy link
Member

jrieken commented Jan 8, 2019

Please fix strict null error first: yarn run strict-null-check-watch

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch from 6bc73d8 to c9a3668 Jan 8, 2019

@jamesgeorge007

This comment has been minimized.

Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

@jrieken It says that string[] can't be assigned to string on line 275 in which the description field is passed as an argument to the Snippet instance.
What's going wrong?

@usernamehw

This comment has been minimized.

Copy link
Contributor

usernamehw commented Jan 8, 2019

What about changing Snippet types to match string | string[]:

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch 2 times, most recently from 9301663 to 13b4726 Jan 8, 2019

@usernamehw

This comment has been minimized.

Copy link
Contributor

usernamehw commented Jan 8, 2019

Wait, it shouldn't be string | string[]. It should stay the same string. You need to change types, so that new Snippet receives only description with type string.

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch 2 times, most recently from f1a59e8 to f7890c7 Jan 8, 2019

@jamesgeorge007

This comment has been minimized.

Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

@usernamehw Here, the description field is converted to string type just as it is done for body before passing it to the Snippet instance.

@usernamehw

This comment has been minimized.

Copy link
Contributor

usernamehw commented Jan 8, 2019

And here should probably be the same check for description as for body:

if ((typeof prefix !== 'string' && !Array.isArray(prefix)) || typeof body !== 'string') {

if ((typeof prefix !== 'string' && !Array.isArray(prefix)) || typeof body !== 'string' || typeof description !== 'string') {
	return;
}

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch 2 times, most recently from 6866bff to 7aa3d38 Jan 8, 2019

@jamesgeorge007

This comment has been minimized.

Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

@usernamehw @jrieken Is it fine now?

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch 4 times, most recently from 55946c7 to 40569ef Jan 8, 2019

@jamesgeorge007

This comment has been minimized.

Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

@jrieken Still the Build is failing!

@jrieken

This comment has been minimized.

Copy link
Member

jrieken commented Jan 8, 2019

@jrieken Still the Build is failing!

Yeah, but that was an unrelated issue... Should be fixed in master now - in case you wanna rebase

@jrieken
Copy link
Member

jrieken left a comment

What's also needed is to update the schema:

description: {
description: nls.localize('snippetSchema.json.description', 'The snippet description.'),
type: 'string'
}

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch from 40569ef to e4f01c6 Jan 8, 2019

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch from d71dedf to b91dfda Jan 8, 2019

@jamesgeorge007

This comment has been minimized.

Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

@jrieken Hope it's fine now 👍

@jamesgeorge007 jamesgeorge007 force-pushed the jamesgeorge007:update-snippets branch from b91dfda to a7149b9 Jan 9, 2019

@jrieken
Copy link
Member

jrieken left a comment

lgtm

@jrieken jrieken merged commit a0ba591 into Microsoft:master Jan 10, 2019

2 checks passed

VS Code #20190109.5 succeeded
Details
license/cla All CLA requirements met.
Details

@jrieken jrieken added this to the December/January 2019 milestone Jan 10, 2019

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