perliedman / leaflet-routing-machine

Control for routing in Leaflet
https://www.liedman.net/leaflet-routing-machine/
Other
1.06k stars 347 forks source link

Doc: Alternative-routing, example code update #658

Closed Kayos113 closed 2 years ago

Kayos113 commented 2 years ago

Hi there,

I noticed in https://www.liedman.net/leaflet-routing-machine/tutorials/alternative-routers/ that a few example codes were difficult to read at first glance. For example, L.Routing.control({ [...] serviceUrl: 'http://my-osrm/route/v1' }); or L.Routing.control({ [...] router: L.Routing.graphHopper('apiKey'); });

A small change to improve readability could be: L.Routing.control({ waypoints: [...], router: L.Routing.graphHopper('apiKey') }); Unless there is something about control instantiation that I do not know yet, as I am posting this as a first time lrm user.

curtisy1 commented 2 years ago

Good point!

I'm guessing the [...] were intended as a ways of abbreviating content as "any available other option(s)". The array syntax can be a bit confusing though, so changing it to what you proposed + a comment like // your other options go here sounds reasonable.

If you want, you can spin up a pull request

Kayos113 commented 2 years ago

That makes sense that [...] was intended to represent all possible other options, though I appreciate your suggestion as I do think it would be an improvement to readability.

Quick question, I have a local repo now, but where do I find the documentation in the stack to make the edit?

curtisy1 commented 2 years ago

You will have to checkout the gh-pages branch. That's the default branch of where the documentation files are located. (In VSCode there should be a button at the bottom-left where you can switch to it. Alternatively, via terminal you could use git checkout gh-pages. You can then find the docs in the tutorials folder.

Kayos113 commented 2 years ago

Thank you for the help, PR just spun up

curtisy1 commented 2 years ago

Thanks for your help and the PR! It just landed on the GitHub pages (PR #662 for future reference)