Fix string cucumber expression parameter completion#476
Fix string cucumber expression parameter completion#476RealByron wants to merge 5 commits intoalexkrechik:masterfrom
Conversation
|
Changelog entry perhaps @RealByron ? |
just done |
|
@alexkrechik is there someone with write access that can take a look at this PR please? |
|
@RobbieDixonBr-dge I'll will review it |
| @@ -1 +1,2 @@ | |||
| this.When('I give 3/4 and 5$') | |||
| this.When('I ask {string} and {string} to {word}') No newline at end of file | |||
There was a problem hiding this comment.
Is the original issue related to the pure steps option enabled? Don't see pureTextSteps: true in the #470 description.
There was a problem hiding this comment.
nope it concerns both values of pureTextSteps
There was a problem hiding this comment.
If this action is not related to the pureTextSteps, then it would be better to add this step to the gserver/test/data/steps/cucumberExpressions.steps.js and add the corresponding test to the or gserver/test/cucumberExpressions.steps.handler.spec.ts or just add test in the describe('getCompletion' test. You could do this or I could make it later in this PR scope.
| @@ -1,3 +1,7 @@ | |||
| ## unreleased | |||
There was a problem hiding this comment.
Minor, but this is not required, I'll add all the info during the release.
| expect(elements.length).to.be.eq(2); | ||
| expect(elements[0].text).to.be.eq('I give 3/4 and 5$'); | ||
| expect(elements[1].text).to.be.eq('I ask {string} and {string} to {word}'); | ||
| console.log(elements) |
There was a problem hiding this comment.
Do we need this console.log?
| } | ||
|
|
||
| // we can replace each ("|')${xx}\\1 by "${xx}" | ||
| res = res.replace(/\(\"\|\'\)(\$\{.+?\})\\1/g, '"$1"'); |
There was a problem hiding this comment.
Maybe move this after the line 360 for consistency? Something like this is already replacing there.
There was a problem hiding this comment.
here it can handle smartSnippets or not ie: "${1:}" or ""
There was a problem hiding this comment.
Yes, I meant to just move this line up to add all the similar actions in one place
There was a problem hiding this comment.
line 360 only concerns replacement when smartSnippets is set to false.
line 364 should be applied regardless of smartSnippets option
|
Happy New Year @alexkrechik @RealByron - not sure what is required to get this ticket moving again? |
|
I'm planning to review everything and make a release this/next week |
|
@alexkrechik bumping the release status? |
3 similar comments
|
@alexkrechik bumping the release status? |
|
@alexkrechik bumping the release status? |
|
@alexkrechik bumping the release status? |
Unfortunately, numerous issues had piled up due to outdated libraries, making local testing of new features impossible. Consequently, I had to address #483. I still need to run auto-tests and retest all scenarios, but the good news is that core functionalities work locally with the newest version of all the vscode-related libs. |
3 participants
fix #470