Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

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

fix #61.
May also help related issues

@jeancroy
Copy link
Contributor Author

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 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 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 commented Feb 3, 2016

Yes, that's what I was thinking.

@jeancroy
Copy link
Contributor Author

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 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

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 pull request Feb 3, 2016
Use a string compatible prefix comparator
@benogle benogle merged commit 9df17f7 into atom:master Feb 3, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sort comparator does not work
3 participants