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

update for spaces/special characters #912

Merged
merged 14 commits into from
Aug 22, 2016
Merged

Conversation

bridgetkromhout
Copy link
Collaborator

As far as I can tell, I've removed "displayname" from being needed at all - I came to the opposite conclusion from what @mattstratton was saying in #861, after digging into this. People can now just set "City" as desired, and the new event script will handle it. The few places I was parsing the city value, I'm now parsing "name" to get the city slug.

It didn't turn out to be necessary to put the city slug in the data file. When updating the new event script as requested in #860, I kept it simple and parsed the entered name instead of asking the user for multiple variations.

I also think I can probably remove "friendly" as a data file element from the sample data file; it's not used, although in one place a variable is also called that.

I'm leaving "current" in the data file for now until it's totally gone and #882 is resolved.

@mattstratton
Copy link
Member

Makes a lot of sense to me.

future.html needs a tweak for this too I think. It has similar conditionals on displayname (although that's just hygiene; sun setting that parameter won't break the code)

@bridgetkromhout
Copy link
Collaborator Author

future.html needs a tweak for this too I think. It has similar conditionals on displayname (although that's just hygiene; sun setting that parameter won't break the code)

Yeah, I know - I was just too tired to resolve the merge conflict last night. :)

@bridgetkromhout
Copy link
Collaborator Author

bridgetkromhout commented Aug 21, 2016

When merged:

Fixes #860
Fixes #858
Fixes #861
Fixes #169
Fixes #922

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.

2 participants