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

Fix string cucumber expression parameter completion #476

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions gclient/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## unreleased
Copy link
Owner

Choose a reason for hiding this comment

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

Minor, but this is not required, I'll add all the info during the release.

* [#470](https://github.com/alexkrechik/VSCucumberAutoComplete/issues/470) Fix autocompletion for string cucumber parameter


## 2.15.3
* Added pureTextSteps option with an ability to use steps as a text (not as a RegExp)
* Did some tables formatting fixes
Expand Down
2 changes: 2 additions & 0 deletions gserver/src/steps.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,8 @@ export default class StepsHandler {
res = res.replace(/\"\[\^\"\]\+\"/g, '""');
}

// we can replace each ("|')${xx}\\1 by "${xx}"
res = res.replace(/\(\"\|\'\)(\$\{.+?\})\\1/g, '"$1"');
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe move this after the line 360 for consistency? Something like this is already replacing there.

Copy link
Author

Choose a reason for hiding this comment

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

here it can handle smartSnippets or not ie: "${1:}" or ""

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, I meant to just move this line up to add all the similar actions in one place

Copy link
Author

Choose a reason for hiding this comment

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

line 360 only concerns replacement when smartSnippets is set to false.
line 364 should be applied regardless of smartSnippets option

return res;
}

Expand Down
2 changes: 1 addition & 1 deletion gserver/test/data/features/after/general.feature
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Feature: Formatting feature
"json": "text"
}
]

"""


@Other
Expand Down
1 change: 1 addition & 0 deletions gserver/test/data/steps/pureTextSteps.steps.js
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
this.When('I give 3/4 and 5$')
this.When('I ask {string} and {string} to {word}')
Copy link
Owner

Choose a reason for hiding this comment

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

Is the original issue related to the pure steps option enabled? Don't see pureTextSteps: true in the #470 description.

Copy link
Author

Choose a reason for hiding this comment

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

nope it concerns both values of pureTextSteps

Copy link
Owner

Choose a reason for hiding this comment

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

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.

33 changes: 20 additions & 13 deletions gserver/test/steps.handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ describe('geStepDefinitionMatch', () => {
});

describe('invalid lines', () => {
const inbvalidStrings = [
const invalidStrings = [
`iGiven('I do something')`,
`Giveni('I do something')`,
`console.log("but i do 'Something'");`
];
inbvalidStrings.forEach(str => {
invalidStrings.forEach(str => {
it(`should not parse "${str}" string`, () => {
const match = s.geStepDefinitionMatch(str);
expect(match).to.be.null;
Expand All @@ -107,7 +107,7 @@ describe('geStepDefinitionMatch', () => {
});

describe('getStepInvariants', () => {
it('should correctly handle or experssions', () => {
it('should correctly handle or expressions', () => {
const str = 'I do (a|b) and then I do (c|d|(?:e|f))';
const res = [
'I do a and then I do c',
Expand Down Expand Up @@ -156,7 +156,7 @@ describe('getRegTextForStep', () => {
['I use {}', 'I use .*'],
['I have 1 cucumber(s) in my belly', 'I have 1 cucumber(s)? in my belly'],
['I have cucumbers in my belly/stomach', 'I have cucumbers in my (belly|stomach)'],
['I use {word}', 'I use [A-Za-z]+']
['I use {word}', 'I use [^\\s]+']
];
data.forEach(d => {
expect(s.getRegTextForStep(d[0])).to.be.equal(d[1]);
Expand Down Expand Up @@ -275,22 +275,22 @@ describe('validate', () => {
it('should not check non-Gherkin steps', () => {
expect(s.validate('Non_gherkin_word do something else', 1, '')).to.be.null;
});
it('should return an diagnostic for lines beggining with Given', () => {
it('should return an diagnostic for lines beginning with Given', () => {
expect(s.validate('Given I do something else', 1, '')).to.not.be.null;
});
it('should return an diagnostic for lines beggining with When', () => {
it('should return an diagnostic for lines beginning with When', () => {
expect(s.validate('When I do something else', 1, '')).to.not.be.null;
});
it('should return an diagnostic for lines beggining with Then', () => {
it('should return an diagnostic for lines beginning with Then', () => {
expect(s.validate('Then I do something else', 1, '')).to.not.be.null;
});
it('should return an diagnostic for lines beggining with And', () => {
it('should return an diagnostic for lines beginning with And', () => {
expect(s.validate('And I do something else', 1, '')).to.not.be.null;
});
it('should return an diagnostic for lines beggining with But', () => {
it('should return an diagnostic for lines beginning with But', () => {
expect(s.validate('But I do something else', 1, '')).to.not.be.null;
});
it('should return an diagnostic for lines beggining with *', () => {
it('should return an diagnostic for lines beginning with *', () => {
expect(s.validate('* I do something else', 1, '')).to.not.be.null;
});
it('should correctly handle outline steps', () => {
Expand Down Expand Up @@ -476,13 +476,20 @@ describe('step as a pure text test', () => {
const elements = customStepsHandler.getElements();

it('should properly handle steps', () => {
expect(elements.length).to.be.eq(1);
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)
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need this console.log?

Copy link
Author

Choose a reason for hiding this comment

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

nope, i will fix that

});

it('should return proper completion', () => {
const completion = customStepsHandler.getCompletion('When I', 1, '');
expect(completion[0].insertText).to.be.eq('I give 3/4 and 5$');
const completion = customStepsHandler.getCompletion('When I g', 1, '');
expect(completion[0].insertText).to.be.eq('give 3/4 and 5$');
})

it('should return proper completion for string snippet', () => {
const completion = customStepsHandler.getCompletion('When I ask ', 1, '');
expect(completion[0].insertText).to.be.eq('"${1:}" and "${2:}" to ${3:}');
})

it('should return proper partial completion', () => {
Expand Down