project-osrm / osrm-text-instructions.swift Goto Github PK

When an on- or off-ramp step includes both destinations and destinationCodes, string(for:) includes the first item in destinations and discards destinationCodes. For example, a route that proceeds onto this way would be announced as:

Take the ramp on the right towards San Francisco

instead, it should match the behavior of the JavaScript version:

Take the ramp on the right towards US 101 North: San Francisco

This discrepancy flew under the radar because only MapboxDirections.swift distinguishes between destinations and destinationCodes via its own parsing – a workaround for Project-OSRM/osrm-text-instructions#41 – and because the JavaScript project lacks a test for a step that includes both a destination route number and destination name.

/cc @bsudekum @ericrwolfe @MoKob

I ran into a problem when I tried to use MapboxDirections, OSRMTextInstructions and MapboxCoreNavigation pods in one project.

The problem is caused by the fact that that there are API changes between MapboxDirections 0.13 and Mapbox Directions 0.16 which aren't backward compatible (despite the fact that according to semantic versioning 0.16 should be backward compatible with 0.13).

Any chance that you can release a new version of osrm text instructions that works with the newest release of MapboxDirections (0.16)?

Podfile

target 'Example' do
  use_frameworks!

  pod 'MapboxDirections.swift'
  pod 'OSRMTextInstructions'
  pod 'MapboxCoreNavigation' 
end

Podfile.lock

PODS:
  - MapboxCoreNavigation (0.13.0):
    - MapboxDirections.swift (~> 0.16.0)
    - MapboxMobileEvents (~> 0.2)
    - Turf (~> 0.0.4)
  - MapboxDirections.swift (0.16.0):
    - Polyline (~> 4.2)
  - MapboxMobileEvents (0.2.10)
  - OSRMTextInstructions (0.6.0):
    - MapboxDirections.swift (~> 0.13)
  - Polyline (4.2.0)
  - Turf (0.0.4)

DEPENDENCIES:
  - MapboxCoreNavigation
  - MapboxDirections.swift
  - OSRMTextInstructions

SPEC CHECKSUMS:
  MapboxCoreNavigation: 3c964ad1854179dc619b4cceb5f85f428b68fe14
  MapboxDirections.swift: 168cc83745b188d792c5482bfb0653c233d1455c
  MapboxMobileEvents: 9e48b59d0eed04508595dcb04a77b7632e1c3054
  OSRMTextInstructions: 7abc90eaf50ff1f7333c526daf360f6a2f1ea059
  Polyline: 3d69f75bb136357e27291d439d0436c6ebda57a4
  Turf: 6c169618fa671e3f8a7b764b5b3332053ad6110e

PODFILE CHECKSUM: 2451d9656bb36e932469bfba9f5797a86fca4499

COCOAPODS: 1.4.0

Hi guys,

I'm planning to use your library in a project of mine that's build with Swift5.x.
Your library doesn't compile with xCode 10.2.1 because of this.

Is there any chance to get an updated version of it?

I'm happy to submit a pull request if it's ok on your side. :-)

Thanks,
Alessandro

Tests take far too long to complete.

/cc @1ec5 @frederoni

An instruction to take an off-ramp should indicate the exit number where available. This would involve porting Project-OSRM/osrm-text-instructions#116 and would depend on mapbox/mapbox-directions-swift#147.

/cc @bsudekum

The podspec being added in #4 claims macOS, tvOS, and watchOS support, but we can’t really be sure those platforms are supported until we build a framework for each of them as part of the Xcode project.

/cc @frederoni

Port the grammar rule functionality added in Project-OSRM/osrm-text-instructions#102, along with the Russian declension rules.

Cannot convert the value of type '[String : Any]?' to expected argument type '[NSAttributedStringKey : Any]?'

OSRMInstructionFormatter should immediately deserialize the JSON instructions into a strongly typed struct. This change would sharply reduce the number of implicitly unwrapped optionals (!) that could cause crashes and clean up the code significantly. Once we migrate to Swift 4, we’ll be able to take advantage of the Codable protocol to reduce the code size even further.

/cc @frederoni @freenerd

If you install using cocoa pods.

pod 'OSRMTextInstructions', :git => 'https://github.com/Project-OSRM/osrm-text-instructions.swift.git'

The Instructions.plist is not included and then you get a crash on the line.

let OSRMTextInstructionsStrings = NSDictionary(contentsOfFile: Bundle(for: OSRMInstructionFormatter.self).path(forResource: "Instructions", ofType: "plist")!)!

The temporary work around is to manually add the Instructions.plist to the pod folder manually. (which is obviously not a viable long term option)

I suspect it got introduced when you moved the plist files into localised folders so the s.resources no longer matches.

s.resources = ['OSRMTextInstructions/*.plist']

/cc @1ec5 @frederoni

CocoaPods is issuing the following warnings when using Xcode 9.1:

 -> OSRMTextInstructions (0.5.0)
    - WARN  | xcodebuild:  OSRMTextInstructions/OSRMTextInstructions/OSRMTextInstructions.swift:18:23: warning: 'characters' is deprecated: Please use String or Substring directly
    - WARN  | xcodebuild:  OSRMTextInstructions/OSRMTextInstructions/OSRMTextInstructions.swift:18:67: warning: 'characters' is deprecated: Please use String or Substring directly
    - WARN  | xcodebuild:  OSRMTextInstructions/OSRMTextInstructions/OSRMTextInstructions.swift:100:102: warning: 'characters' is deprecated: Please use String or Substring directly

These issues would be resolved by migrating to Swift 4.

/cc @bsudekum

This to-do (“add correct waypoint counting”) should be fixed by porting Project-OSRM/osrm-text-instructions#111.

/cc @bsudekum

The CocoaPods podspec should define subspecs for individual locales, so that a size-conscious developer can pick-and-choose the locales that their application supports.

To avoid hard-coding each supported locale in this repository – one of the goals of #14 – we could provide a subspec for English, one for the Chinese locales, one for Western European locales, and one for all remaining languages. By default, all the subspecs would be included.

In each subspec, json2plist.sh would accept locale codes as arguments and convert only the specified locales if provided.

/cc @frederoni @bsudekum @freenerd

Hi guys,

in OSRMInstructionFormatter I found this:

if type != .depart && type != .arrive && modifier == .none { return nil }
it looks like if the step's type is, for instance, .turn the library doesn't return an attributed string that describes the step.
Can you enlighten me about the reasons behind this? :-)
In my use case it's important to explain the user the step-by-step instructions of the route it's gonna take.

Thanks,
Alessandro

This library should pull translations directly from Transifex and convert them into plist format to complement the existing English plist. This should be done as a build step to keep developers from having to access the Transifex API every time they run carthage bootstrap.

/cc @frederoni @freenerd

The iOS navigation SDK currently defines its own format strings for situations that aren’t adequately covered by this library:

https://github.com/mapbox/mapbox-navigation-ios/blob/55417885cc87e9fa28a84780d35db3a737878a17/MapboxCoreNavigation/Resources/en.lproj/Localizable.strings#L1-L26

Project-OSRM/osrm-text-instructions#141 added analogous strings, so we should port that work to this library and remove the strings and the associated logic from the iOS navigation SDK.

/cc @mcwhittemore @bsudekum