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

Grammar notes update for Swift implementation #176

Merged
merged 2 commits into from
Nov 3, 2017

Conversation

yuryleb
Copy link
Contributor

@yuryleb yuryleb commented Oct 16, 2017

This PR updates Grammar feature description with notes about OSRM Text Instructions for Swift implementation specifics - always global matching regardless of specified regular expressions flags.

Some Russian regular expressions are also joint and optimized to work better in Swift too.

@yuryleb yuryleb force-pushed the grammar-notes-update branch 2 times, most recently from a0b7a76 to 05290ed Compare October 20, 2017 08:40
@mcwhittemore
Copy link
Contributor

@1ec5 can you take a look at this?

@yuryleb yuryleb force-pushed the grammar-notes-update branch 3 times, most recently from bdb7efa to f1a5114 Compare October 26, 2017 21:03
Copy link
Member

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

This looks alright to me on a technical level. @jcsg, could I interest you in reviewing the changes to the grammar rules?

@@ -28,9 +28,21 @@ The required grammatical case should be specified right in instruction's substit
- Instruction text formatter ([index.js](index.js) in this module) should:
- check `{way_name}` and `{rotary_name}` variables for optional grammar case after colon: `{way_name:accusative}`
- find appropriate regular expressions block for target language and specified grammar case
- call standard [string replace with regular expression](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace) for each expression in block passing result from previous call to the next; the first call should enclose original street name with whitespaces to make parsing words in names a bit simplier.
- call standard [string replace with regular expression](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace) for each expression in block passing result from previous call to the next; the first call should enclose original street name with whitespaces to make parsing several words inside name a bit simplier.
Copy link
Member

Choose a reason for hiding this comment

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

Is this still a requirement, now that grammarize is called before capitalization and insertion (#170)?

@jcsg
Copy link

jcsg commented Nov 3, 2017

@1ec5 Yes, the changes to the grammar rules look good to me.

@1ec5 1ec5 merged commit d2709c4 into Project-OSRM:master Nov 3, 2017
@1ec5
Copy link
Member

1ec5 commented Nov 3, 2017

Thanks @yuryleb!

@yuryleb yuryleb deleted the grammar-notes-update branch November 3, 2017 18:32
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.

None yet

4 participants