Skip to content

yaml completion: add a space between ':' and value#245

Merged
JPinkney merged 4 commits intoredhat-developer:masterfrom
fbaligand:master
Mar 29, 2020
Merged

yaml completion: add a space between ':' and value#245
JPinkney merged 4 commits intoredhat-developer:masterfrom
fbaligand:master

Conversation

@fbaligand
Copy link
Copy Markdown
Contributor

Fix the missing space between ':' and value in YAML completion, especially for default values, but not only.
Fix issue redhat-developer/vscode-yaml#281

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 21, 2020

Coverage Status

Coverage remained the same at 77.275% when pulling 2d8f356 on fbaligand:master into 62fa41c on redhat-developer:master.

@fbaligand
Copy link
Copy Markdown
Contributor Author

@JPinkney
Concerning the failed checks:

  • CI build: first job just didn't run because an error occurred while generating the build script.
  • But CI build second job run successfully!
  • I'm surprised that coverage has decreased, given that there is no added code line, just trailing spaces.

@gorkem gorkem requested a review from JPinkney March 22, 2020 15:06
@JPinkney
Copy link
Copy Markdown
Contributor

@fbaligand The PR looks good to me! Do you mind writing a couple small tests so that we can make sure this issue doesn't happen again

@fbaligand
Copy link
Copy Markdown
Contributor Author

Ok to add a test for default value.
What is best existing .ts test file where it would be the more relevant?

@JPinkney
Copy link
Copy Markdown
Contributor

Any of autocompletion ones should work. The only difference between them is the schemas that they use. If none of them can be used (if they don't provide a schema that allows you to call the correct sections of the code) you can do what autoCompletion5 did and manually create a schema in the fixtures folder and just create autoCompletion6.ts. The whole testing structure of those files needs to be revamped so I'd be ok to just add another file if needed

@fbaligand
Copy link
Copy Markdown
Contributor Author

Hi,

Well, I'm quite bothered and need help for my unit test...
Actually, I added a test, trying to do exactly as other unit tests.
My test is inspired by autoCompletion.test.ts and autoCompletion5.test.ts.
But despite that, the result is not at all what I expect, and I trully don't understand why...

I expect to have only one result item, that is 'directory' entry with directory: bower_components completion.
But for real, I get all of the 19 possible yaml entries, starting with analytics.

So can you tell me what to do so that my test works?
Or directly fix it in my branch?

Thanks in advance. I can assure you that I spent some time to try to make this test work.
I ask for your help, really because I don't understand and don't see a solution.

@JPinkney
Copy link
Copy Markdown
Contributor

I think what you can do for now is:

assert.equal(result.items[2].insertText, 'directory: ${1:bower_components}');

and the tests will start working. It looks like during the completion request everything gets sent back to the client and its not until the completion resolve request that it narrows down to directory

@fbaligand
Copy link
Copy Markdown
Contributor Author

Hi @JPinkney,

Thanks a lot for your help!
I applied your tip and it works great!
Now all tests are OK.

Tell me if there is something else to add/change.

@JPinkney JPinkney merged commit b9a8f2d into redhat-developer:master Mar 29, 2020
@fbaligand
Copy link
Copy Markdown
Contributor Author

Great!
Thanks @JPinkney for the merge!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants