Skip to content
This repository was archived by the owner on Jan 28, 2020. It is now read-only.

Conversation

ShawnMilo
Copy link
Contributor

Fixed display of vocabulary terms containing spaces.

fixes #367

I spent some (too much) time on the command line using the elasticsearch
module trying to figure out how to fix the search to work with spaces.

In the end, I went for the obvious, lame solution. Eventually it seems
like we'll want to ditch Haystack and use Elasticsearch directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes belong to commit #377; hopefully it'll be merged before this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

hypen -> hyphen :)

fixes #376

Also explicitly sorts instead of depending on chance.
fixes #367

I spent some (too much) time on the command line using the elasticsearch
module trying to figure out how to fix the search to work with spaces.

In the end, I went for the obvious, lame solution. Eventually it seems
like we'll want to ditch Haystack and use Elasticsearch directly.
@ShawnMilo ShawnMilo force-pushed the bug/skm/367_term_spaces branch from 967e856 to cafe20c Compare July 17, 2015 20:15
@giocalitri
Copy link
Contributor

This look good to me.
I tested the functionality and it solves the space problem, but it does not solve the case where the therm is something like anc@ra: it splits on the @ and there are 2 facets anc and ra

@pdpinch agreed to merge as it is and open another issue

giocalitri added a commit that referenced this pull request Jul 17, 2015
Fixed display of vocabulary terms containing spaces.
@giocalitri giocalitri merged commit 885bd0b into master Jul 17, 2015
@giocalitri giocalitri deleted the bug/skm/367_term_spaces branch July 17, 2015 21:02
@ShawnMilo
Copy link
Contributor Author

@giocalitri #384 added. Perhaps base64 encoding will be the silver bullet.

@Ferdi
Copy link
Contributor

Ferdi commented Jul 17, 2015

Without complicating this further, I suggest we only allow users to provide alphanumeric term names

@giocalitri
Copy link
Contributor

@Ferdi better to continue the discussion on #384

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.

4 participants