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

Made UI respect order of attributes in config #1102

Merged
merged 4 commits into from
Nov 6, 2014

Conversation

duncanka
Copy link
Contributor

@duncanka duncanka commented Aug 1, 2014

I found that for my annotation project, annotators wanted to see the attribute options in a specific order (they were on a spectrum). This change wasn't a particularly drastic one, and it seemed like it might be useful to others, as well.

@ghost
Copy link

ghost commented Nov 4, 2014

This one is also awesome, why hasn't it been merged?

@ghost ghost added this to the v1.4 Lemon Curry milestone Nov 4, 2014
@ghost ghost assigned amadanmath Nov 4, 2014
@ghost ghost added the comp-server label Nov 4, 2014
@ghost
Copy link

ghost commented Nov 4, 2014

The server-side changes look fine, as much as I hate pre-allocation, would have opted for sorting a list of tuples, I am willing to merge the changes. @amadanmath: Do you have any opinion on the client-side changes?

@duncanka
Copy link
Contributor Author

duncanka commented Nov 5, 2014

I could certainly update it to avoid pre-allocation, if you'd prefer...

@ghost
Copy link

ghost commented Nov 5, 2014

@duncanka: It would certainly be appreciated. Pre-allocation can lead to nasty bugs further down the line.

@duncanka
Copy link
Contributor Author

duncanka commented Nov 6, 2014

Done.

It's now displaying that there are merge conflicts; I think I may have accidentally created them by letting Emacs strip whitespace. If those are the source of the conflicts, sorry about that.

amadanmath pushed a commit that referenced this pull request Nov 6, 2014
Made UI respect order of attributes in config
@amadanmath amadanmath merged commit dc7cecc into nlplab:master Nov 6, 2014
@amadanmath
Copy link
Contributor

Stripping whitespace is not a problem, just beware of sneaky tabs. :) Thanks for the contrib!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants