Skip to content
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

Bug on auto-completion: default value is inserted just after semi-colon (instead of one space after) #281

Closed
fbaligand opened this issue Mar 8, 2020 · 8 comments

Comments

@fbaligand
Copy link
Contributor

fbaligand commented Mar 8, 2020

Say that I have this json schema:

		"pipeline.id": {
			"type": "string",
			"description": "The ID of the pipeline.",
			"default": "main"
		},

If I use auto-completion in my YAML file, typing pipeline, then press <TAB> key, I get this entry in my YAML file:
pipeline.id:main

The expected generated YAML entry should be:
pipeline.id: main

Could you fix this little bug?

@fbaligand fbaligand changed the title Bug: on auto-completion, default value is inserted just after semi-colon (instead of one space after) Bug on auto-completion: default value is inserted just after semi-colon (instead of one space after) Mar 8, 2020
@JPinkney
Copy link
Contributor

@fbaligand
Copy link
Contributor Author

Thanks for your analysis and pointed line!
Indeed, I think that bug is around there.
But more at line 693. Because, later some “value” variable affectations have a “ “ prefix, like line 719.

what do you think about?

@JPinkney
Copy link
Contributor

Yep, looks like most likely the spaces on 719, 722, 732, 735 need to be removed

@fbaligand
Copy link
Contributor Author

Yes, this would be a nice way to fix!
The only (small) disadvantage of this way would be that when there is no simple value, but a line return and child properties, this would generate a "useless" space between : and \n.
I think about lines 704, 706, 725 and 728.

The other way is to add a ' ' prefix at lines 676, 687, 693, 699 and 743.

Which way do you prefer?

@fbaligand
Copy link
Contributor Author

@JPinkney Tell me the way that you prefer, and then, I'm ready to submit a PR.

@JPinkney
Copy link
Contributor

I think try the second way:

The other way is to add a ' ' prefix at lines 676, 687, 693, 699 and 743.

and then if that doesn't work out try the first way

@fbaligand
Copy link
Contributor Author

Ok, I will do so.

@fbaligand
Copy link
Contributor Author

@JPinkney

I just submitted the PR:
redhat-developer/yaml-language-server#245

I tested on my vscode, it works as expected for me!

Can you review the PR?

bleach31 pushed a commit to bleach31/vscode-yaml that referenced this issue Jan 25, 2022
…ce-folders

Handle workspace folders better
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants