> On July 16, 2016, 12:32 p.m., Sandro Knauß wrote: > > As I saw inside my browser - marble registers to handle the geo links. As I > > read somewhere you can also use marble to include a map / so maybe you > > should connect the marble guys for that ( but this is only an improvement) > > nothing that holds back this review.
I thought about marble too, but Image and Video messages would have much bigger priority for me. > On July 16, 2016, 12:32 p.m., Sandro Knauß wrote: > > filters/geopoint/CMakeLists.txt, line 4 > > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471820#file471820line4> > > > > use same indent format in a file either a static indent or indent with ( I'm agree with the issue, but I copied this from "highlight" filter. For some (unknown for me) reason we have such indent in all other filters, so I just followed the style. Do we have some CMake coding convension somewhere? I changed the style to the static four spaces indentation. > On July 16, 2016, 12:32 p.m., Sandro Knauß wrote: > > filters/geopoint/geopoint-filter.cpp, line 62 > > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471822#file471822line62> > > > > is this compiling with the ; ? Yes. Actually, it does *not* compiles without the colon. We have this in all filters. > On July 16, 2016, 12:32 p.m., Sandro Knauß wrote: > > filters/geopoint/geopoint-filter.cpp, line 42 > > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471822#file471822line42> > > > > osm using this improved geo link style (if you use the share button at > > the right panel at openstreetmap.org): > > > > geo:50.8295,12.9225?z=16 > > > > so add at the end to match the zoom property: > > > > (?z=(?<zoom>\d+))? > > > > and replace all [0-9] with \d or is there any need to mix [0-9] and \d ? > > > > for sure you need to add the logic to set the zoom level to the iframe, > > too. I thought to do it later, but ok, just done. > On July 16, 2016, 12:32 p.m., Sandro Knauß wrote: > > filters/geopoint/geopoint-filter.cpp, line 38 > > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471822#file471822line38> > > > > <a href="geo:%1,%2?z=%3">geo:%1,%2?z=%3</a><iframe>...</iframe> > > > > a user still can see the geo link, that was sent. and can open/copy it > > like he wants. Good idea to add a href, but, as I see (just tried it), it would be better to replace a geo text with geo link and add the iframe(s) to the end of message, rather than add href to the end. - Alexandr ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128460/#review97460 ----------------------------------------------------------- On July 16, 2016, 7:04 a.m., Alexandr Akulich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128460/ > ----------------------------------------------------------- > > (Updated July 16, 2016, 7:04 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-text-ui > > > Description > ------- > > Added a geopoint filter which adds an iframe with a map for each > geo:<double,double> in message (RFC 5870). > Uses OpenStreetMap as "backend". > > > Diffs > ----- > > filters/CMakeLists.txt 8118b13 > filters/geopoint/CMakeLists.txt PRE-CREATION > filters/geopoint/geopoint-filter.h PRE-CREATION > filters/geopoint/geopoint-filter.cpp PRE-CREATION > filters/geopoint/ktptextui_message_filter_geopoint.desktop.cmake > PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/128460/diff/ > > > Testing > ------- > > It works for me. > > > Thanks, > > Alexandr Akulich > >
