Skip to content

Add multiline description support in user snippets#66159

Merged
jrieken merged 1 commit intomicrosoft:masterfrom
jamesgeorge007:update-snippets
Jan 10, 2019
Merged

Add multiline description support in user snippets#66159
jrieken merged 1 commit intomicrosoft:masterfrom
jamesgeorge007:update-snippets

Conversation

@jamesgeorge007
Copy link
Contributor

@jamesgeorge007 jamesgeorge007 commented Jan 7, 2019 •
edited
Loading

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"
},

usernamehw
usernamehw reviewed Jan 7, 2019
View reviewed changes
@jamesgeorge007 jamesgeorge007 force-pushed the update-snippets branch 2 times, most recently from 963103e to 6bc73d8 Compare January 8, 2019 02:56
@jrieken
Copy link
Member

jrieken commented Jan 8, 2019

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

@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019 •
edited
Loading

@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
Copy link
Contributor

usernamehw commented Jan 8, 2019

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

https://github.com/Microsoft/vscode/blob/c9a3668a88552167d8cb88424ba7cd37039b3a53/src/vs/workbench/parts/snippets/electron-browser/snippetsFile.ts#L28

@jamesgeorge007 jamesgeorge007 force-pushed the update-snippets branch 2 times, most recently from 9301663 to 13b4726 Compare January 8, 2019 10:04
@usernamehw
Copy link
Contributor

usernamehw commented Jan 8, 2019

https://github.com/Microsoft/vscode/blob/c9a3668a88552167d8cb88424ba7cd37039b3a53/src/vs/workbench/parts/snippets/electron-browser/snippetsFile.ts#L28

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 update-snippets branch 2 times, most recently from f1a59e8 to f7890c7 Compare January 8, 2019 10:16
@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

https://github.com/Microsoft/vscode/blob/f7890c7ad7bf0e0d25cefb9e3033cc6b98312fce/src/vs/workbench/parts/snippets/electron-browser/snippetsFile.ts#L235

@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
usernamehw reviewed Jan 8, 2019
View reviewed changes
@usernamehw
Copy link
Contributor

usernamehw commented Jan 8, 2019

And here should probably be the same check for description as for body:
https://github.com/Microsoft/vscode/blob/f7890c7ad7bf0e0d25cefb9e3033cc6b98312fce/src/vs/workbench/parts/snippets/electron-browser/snippetsFile.ts#L239

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

@jamesgeorge007 jamesgeorge007 force-pushed the update-snippets branch 2 times, most recently from 6866bff to 7aa3d38 Compare January 8, 2019 10:36
@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

@usernamehw @jrieken Is it fine now?

jrieken
jrieken requested changes Jan 8, 2019
View reviewed changes
@jamesgeorge007 jamesgeorge007 force-pushed the update-snippets branch 4 times, most recently from 55946c7 to 40569ef Compare January 8, 2019 12:50
@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019 •
edited
Loading

@jrieken Still the Build is failing!

@jrieken
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
jrieken requested changes Jan 8, 2019
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's also needed is to update the schema: https://github.com/Microsoft/vscode/blob/5aba426b77d58dbeedbc539d1780931b917cb8bb/src/vs/workbench/parts/snippets/electron-browser/snippets.contribution.ts#L52-L55

@jrieken jrieken added feature-request Request for new features or functionality snippets labels Jan 8, 2019
@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented Jan 8, 2019

@jrieken Hope it's fine now 👍

jrieken
jrieken approved these changes Jan 10, 2019
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jrieken jrieken merged commit a0ba591 into microsoft:master Jan 10, 2019
@jrieken jrieken added this to the December/January 2019 milestone Jan 10, 2019
@jrieken jrieken added verification-needed Verification of issue is requested release-notes Release notes issues labels Jan 28, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
@jamesgeorge007 jamesgeorge007 deleted the update-snippets branch November 21, 2024 07:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jrieken jrieken jrieken approved these changes

+1 more reviewer

@usernamehw usernamehw usernamehw left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jrieken jrieken

Labels

feature-request Request for new features or functionality release-notes Release notes issues snippets verification-needed Verification of issue is requested

Projects

None yet

Milestone

December/January 2019

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add multiline description support in user snippets

3 participants

@jamesgeorge007 @jrieken @usernamehw