Skip to content

fix completion error#337

Merged
jcubic merged 2 commits intojcubic:develfrom
avdes:patch-1
Sep 20, 2017
Merged

fix completion error#337
jcubic merged 2 commits intojcubic:develfrom
avdes:patch-1

Conversation

@avdes
Copy link
Copy Markdown
Contributor

@avdes avdes commented Sep 19, 2017

for example: https://codepen.io/a_vasilev/pen/mBPOjL
open and press "TAB"

@jcubic
Copy link
Copy Markdown
Owner

jcubic commented Sep 19, 2017

thanks for the fix, but please modify jquery.terminal-src.js file (that's the source file - just copy paste the code to the same place) and create PR against devel branch (that one you can change by clicking edit). you can also run make to create output files, then you will be mark as contributor to output files as well.

Also fix the indent it's 4 spaces more for the whole for statement, the travis didn't throw error because it check only src file, you can check by running (after modifing -src.js file):

npm install
make lint

from terminal

After indent fix you will have one line too long, the expected 90 characters eslint rule, so you need to make this line shorter:

} else if (pushCandidate.toLowerCase() === currentChar.toLowerCase()) {

you can fix that by changing names to chr or current and candidate.

And thanks again for the fix

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 80.167% when pulling 373233a on avdes:patch-1 into 965b795 on jcubic:master.

@avdes
Copy link
Copy Markdown
Contributor Author

avdes commented Sep 19, 2017

Thank`s. Should I create a new pull request?

change var names
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.1%) to 80.053% when pulling 3e08483 on avdes:patch-1 into 965b795 on jcubic:master.

@jcubic jcubic changed the base branch from master to devel September 20, 2017 07:17
@jcubic
Copy link
Copy Markdown
Owner

jcubic commented Sep 20, 2017

I've changed branch to devel.

@jcubic jcubic merged commit 484866d into jcubic:devel Sep 20, 2017
@jcubic
Copy link
Copy Markdown
Owner

jcubic commented Dec 24, 2017

There was bug in your PR found in #361 there was needed break from outer for loop if push is false

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