The Wayback Machine - https://web.archive.org/web/20220404053851/https://github.com/atom/autocomplete-snippets/pull/66
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

Use a string compatible prefix comparator #66

Merged
merged 4 commits into from Feb 3, 2016
Merged

Use a string compatible prefix comparator #66

merged 4 commits into from Feb 3, 2016

Conversation

jeancroy
Copy link
Contributor

@jeancroy jeancroy commented Jan 22, 2016

fix #61.
May also help related issues

@jeancroy
Copy link
Contributor Author

@jeancroy jeancroy commented Feb 3, 2016

/cc @50Wliu because I think this would help the ruby do/dop snippet situation.

I'm not sure if spec are needed or how I'd write one.

@lee-dohm
Copy link
Contributor

@lee-dohm lee-dohm commented Feb 3, 2016

@jeancroy Specs are strongly preferred. Basically I would just write a bunch of tests describing sorting cases and the expected output. Extra credit for covering failure cases like people handing undefined, null or non-string input.

@jeancroy
Copy link
Contributor Author

@jeancroy jeancroy commented Feb 3, 2016

@lee-dohm would it be OK to test the sortComparator in isolation ?
I can give an array of unsorted data, and try to sort using this.

it's just the inner working of autocomplete-snippet i'm not sure. It looks like a glue package between the snippet package and autocomplete-plus.

@lee-dohm
Copy link
Contributor

@lee-dohm lee-dohm commented Feb 3, 2016

Yes, that's what I was thinking.

@jeancroy
Copy link
Contributor Author

@jeancroy jeancroy commented Feb 3, 2016

Thanks @lee-dohm, for pushing for test, I blindly assumed at least the a.prefix part was right. It looks like result don't have a prefix property but a text one.

@lee-dohm
Copy link
Contributor

@lee-dohm lee-dohm commented Feb 3, 2016

You're welcome 👍

@@ -71,3 +71,27 @@ describe 'AutocompleteSnippets', ->
runs ->
atom.commands.dispatch(editorView, 'autocomplete-plus:confirm')
expect(editor.getText()).toContain '} while (true);'

Copy link
Contributor

@benogle benogle Feb 3, 2016

Choose a reason for hiding this comment

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

🔥 this newliine

benogle added a commit that referenced this issue Feb 3, 2016
Use a string compatible prefix comparator
@benogle benogle merged commit 9df17f7 into atom:master Feb 3, 2016
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants