Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review29923 --- This review has been submitted with commit 1121cff1b8600079909424a90923f75b5c3630ad by Albert Astals Cid on behalf of Jaydeep Solanki to branch master. - Commit Hook On March 25, 2013, 7:51 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 25, 2013, 7:51 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - tests/CMakeLists.txt 6a26b3e tests/urldetecttest.cpp PRE-CREATION ui/pageview.cpp e8d481d ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 26, 2013, 9:49 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - tests/CMakeLists.txt 6a26b3e tests/urldetecttest.cpp PRE-CREATION ui/pageview.cpp e8d481d ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 25, 2013, 6:56 p.m.) Review request for Okular. Changes --- updated Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - tests/CMakeLists.txt 6a26b3e tests/urldetecttest.cpp PRE-CREATION ui/pageview.cpp e8d481d ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 25, 2013, 7:51 p.m.) Review request for Okular. Changes --- moved the prepending of http to getUrl method. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - tests/CMakeLists.txt 6a26b3e tests/urldetecttest.cpp PRE-CREATION ui/pageview.cpp e8d481d ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review29342 --- Does KRun know how to run www.google.com ? Doesn't seem to work here, maybe you should prepend http:// if there is no protocol in the url? - Albert Astals Cid On March 15, 2013, 7:28 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 15, 2013, 7:28 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - tests/CMakeLists.txt 6a26b3e tests/urldetecttest.cpp PRE-CREATION ui/pageview.cpp e8d481d ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review29219 --- tests/urldetecttest.cpp http://git.reviewboard.kde.org/r/108614/#comment21813 Your pdf in http://goo.gl/wBM6p had some more complex urls, any reason you didn't add them here? - Albert Astals Cid On March 13, 2013, 2:38 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 13, 2013, 2:38 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - tests/CMakeLists.txt 6a26b3e tests/urldetecttest.cpp PRE-CREATION ui/pageview.cpp b018dfe ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 13, 2013, 2:28 p.m.) Review request for Okular. Changes --- a unit test added. BTW, can I add that banner( that is on top of every file containing copyright ), with my name ? Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - ui/pageview.cpp b018dfe tests/urldetecttest.cpp PRE-CREATION tests/CMakeLists.txt 6a26b3e ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 13, 2013, 2:38 p.m.) Review request for Okular. Changes --- Sorry, the previous one had a typo. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - tests/CMakeLists.txt 6a26b3e tests/urldetecttest.cpp PRE-CREATION ui/pageview.cpp b018dfe ui/url_utils.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 10, 2013, 12:27 a.m., Azat Khuzhin wrote: Seems more appropriate for me. But one more thing, is reg1 still needed? Jaydeep Solanki wrote: Yes, because it prevents asdfhttp://www.google.com; from getting detected. I think that for this it is not necessary. Because of \b in you first regexp. - Azat --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28857 --- On March 9, 2013, 8:13 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 9, 2013, 8:13 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 10, 2013, 12:27 a.m., Azat Khuzhin wrote: Seems more appropriate for me. But one more thing, is reg1 still needed? Jaydeep Solanki wrote: Yes, because it prevents asdfhttp://www.google.com; from getting detected. Azat Khuzhin wrote: I think that for this it is not necessary. Because of \b in you first regexp. Jaydeep Solanki wrote: No. '\b' detects word boundry. For example, in asdfhttp://www.google.com;, '/' is not considered as part of a word, so the first 'w' in 'www.' will be considered as boundary, thus detecting 'www.google.com' as a url, which should not be detected. A better way to do this is lookbehind assertion, but Qt doesn't support it, so I finally decided to use two regexs. Yes, it make sense. BTW qt5 support it https://bugreports.qt-project.org/browse/QTBUG-2371. Maybe you want to leave a TODO for qt5? - Azat --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28857 --- On March 9, 2013, 8:13 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 9, 2013, 8:13 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 10, 2013, 12:27 a.m., Azat Khuzhin wrote: Seems more appropriate for me. But one more thing, is reg1 still needed? Yes, because it prevents asdfhttp://www.google.com; from getting detected. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28857 --- On March 9, 2013, 8:13 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 9, 2013, 8:13 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 10, 2013, 12:27 a.m., Azat Khuzhin wrote: Seems more appropriate for me. But one more thing, is reg1 still needed? Jaydeep Solanki wrote: Yes, because it prevents asdfhttp://www.google.com; from getting detected. Azat Khuzhin wrote: I think that for this it is not necessary. Because of \b in you first regexp. No. '\b' detects word boundry. For example, in asdfhttp://www.google.com;, '/' is not considered as part of a word, so the first 'w' in 'www.' will be considered as boundary, thus detecting 'www.google.com' as a url, which should not be detected. A better way to do this is lookbehind assertion, but Qt doesn't support it, so I finally decided to use two regexs. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28857 --- On March 9, 2013, 8:13 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 9, 2013, 8:13 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 10, 2013, 12:27 a.m., Azat Khuzhin wrote: Seems more appropriate for me. But one more thing, is reg1 still needed? Jaydeep Solanki wrote: Yes, because it prevents asdfhttp://www.google.com; from getting detected. Azat Khuzhin wrote: I think that for this it is not necessary. Because of \b in you first regexp. Jaydeep Solanki wrote: No. '\b' detects word boundry. For example, in asdfhttp://www.google.com;, '/' is not considered as part of a word, so the first 'w' in 'www.' will be considered as boundary, thus detecting 'www.google.com' as a url, which should not be detected. A better way to do this is lookbehind assertion, but Qt doesn't support it, so I finally decided to use two regexs. Azat Khuzhin wrote: Yes, it make sense. BTW qt5 support it https://bugreports.qt-project.org/browse/QTBUG-2371. Maybe you want to leave a TODO for qt5? Oh, I didn't know that, thanks! I'll make sure, this gets improved when kdelibs move to qt5. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28857 --- On March 9, 2013, 8:13 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 9, 2013, 8:13 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 9, 2013, 8:13 p.m.) Review request for Okular. Changes --- How about this? Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28579 --- It trims up to ascii characters. Example: http://google.com/ф; - http://google.com/; - Azat Khuzhin On March 5, 2013, 1:26 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 5, 2013, 1:26 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 5, 2013, 1:26 p.m.) Review request for Okular. Changes --- this should fix it. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 3, 2013, 4:47 p.m., Albert Astals Cid wrote: This is still detecting http://google.com) as something you can call open link on. I'm not sure if http://google.com) is valid or not, but when you click on the Go got 'http://google.com)' I am getting a Malformed URL error. Can you please investigate if 'http://google.com)' is supposed to be a correct URL or not? If it is maybe you can try to find out why the Malformed URL error happens? http://google.com) is not a valid URL. Because host/port can't contain ) and path must start from / see http://www.ietf.org/rfc/rfc3986.txt Also I have a question, why you are using regular expression for this, I don't think that it is easy to write such regexp. Why you don't just split by [\t\n ] and validate URL using qt/kde method? (I think qt/kde must have such) But if there is no such, here is a good example of regexp for URL's - http://stackoverflow.com/a/190405 Also see note about u modifier. - Azat --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28459 --- On March 2, 2013, 10:39 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 2, 2013, 10:39 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28482 --- http://google.com) is not a valid URL. Because host/port can't contain ) and path must start from / see http://www.ietf.org/rfc/rfc3986.txt Also I have a question, why you are using regular expression for this, I don't think that it is easy to write such regexp. Why you don't just split by [\t\n ] and validate URL using qt/kde method? (I think qt/kde must have such) But if there is no such, here is a good example of regexp for URL's - http://stackoverflow.com/a/190405 Also see note about u modifier. - Azat Khuzhin On March 2, 2013, 10:39 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 2, 2013, 10:39 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28459 --- This is still detecting http://google.com) as something you can call open link on. I'm not sure if http://google.com) is valid or not, but when you click on the Go got 'http://google.com)' I am getting a Malformed URL error. Can you please investigate if 'http://google.com)' is supposed to be a correct URL or not? If it is maybe you can try to find out why the Malformed URL error happens? - Albert Astals Cid On March 2, 2013, 10:39 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 2, 2013, 10:39 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 1, 2013, 6:33 p.m., Albert Astals Cid wrote: Why the exceptions? Looks a bit weird to me. Please elaborate. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28357 --- On Feb. 24, 2013, 1:55 a.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 24, 2013, 1:55 a.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On March 1, 2013, 6:33 p.m., Albert Astals Cid wrote: Why the exceptions? Looks a bit weird to me. Jaydeep Solanki wrote: Please elaborate. Why would you want to detect google.com) as a url? - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28357 --- On Feb. 24, 2013, 1:55 a.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 24, 2013, 1:55 a.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 2, 2013, 10:33 p.m.) Review request for Okular. Changes --- I don't know why but after seeing the regex used by konversation I was convinced that it is a good idea, but now I thing it isn't. Anyways, not detecting xyz.com will make it less error prone. Updated. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated March 2, 2013, 10:39 p.m.) Review request for Okular. Description (updated) --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. pdf with links update : http://goo.gl/wBM6p This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp b018dfe Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review28357 --- Why the exceptions? Looks a bit weird to me. - Albert Astals Cid On Feb. 24, 2013, 1:55 a.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 24, 2013, 1:55 a.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 24, 2013, 1:55 a.m.) Review request for Okular. Changes --- regex updated, pdf containing test cases : http://goo.gl/wBM6p Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On Feb. 21, 2013, 3:29 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2185 http://git.reviewboard.kde.org/r/108614/diff/3/?file=114545#file114545line2185 This regexp still needs some tweaking, right now if i pass holahttps://okular.org; it returns https://okular.org that in my opinion is not correct. You should enforce spaces (or newlines or tabs) around the regexp to make sure the url is a single word. Also it would be cool if you could write down all the urls you've used to positively and negatively test the regexp so we can put it in a testcase Jaydeep Solanki wrote: You mean holahttps://okular.org; should not be detected as an url, hola https://okular.org; should be. Am I right ?? Albert Astals Cid wrote: Exactly Mailson Menezes wrote: Try to have a look on how konversation extracts url from strings. More precisely the function extractUrlData in common.cpp thanks, it helped a lot. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review27846 --- On Feb. 24, 2013, 1:55 a.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 24, 2013, 1:55 a.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On Feb. 21, 2013, 3:29 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2183 http://git.reviewboard.kde.org/r/108614/diff/3/?file=114545#file114545line2183 Mere looks, please put the static on front oops! On Feb. 21, 2013, 3:29 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2185 http://git.reviewboard.kde.org/r/108614/diff/3/?file=114545#file114545line2185 This regexp still needs some tweaking, right now if i pass holahttps://okular.org; it returns https://okular.org that in my opinion is not correct. You should enforce spaces (or newlines or tabs) around the regexp to make sure the url is a single word. Also it would be cool if you could write down all the urls you've used to positively and negatively test the regexp so we can put it in a testcase You mean holahttps://okular.org; should not be detected as an url, hola https://okular.org; should be. Am I right ?? - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review27846 --- On Feb. 20, 2013, 2:21 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 20, 2013, 2:21 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On Feb. 21, 2013, 3:29 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2185 http://git.reviewboard.kde.org/r/108614/diff/3/?file=114545#file114545line2185 This regexp still needs some tweaking, right now if i pass holahttps://okular.org; it returns https://okular.org that in my opinion is not correct. You should enforce spaces (or newlines or tabs) around the regexp to make sure the url is a single word. Also it would be cool if you could write down all the urls you've used to positively and negatively test the regexp so we can put it in a testcase Jaydeep Solanki wrote: You mean holahttps://okular.org; should not be detected as an url, hola https://okular.org; should be. Am I right ?? Exactly - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review27846 --- On Feb. 20, 2013, 2:21 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 20, 2013, 2:21 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review27875 --- - Mailson Menezes On Feb. 20, 2013, 2:21 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 20, 2013, 2:21 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On Feb. 21, 2013, 3:29 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2185 http://git.reviewboard.kde.org/r/108614/diff/3/?file=114545#file114545line2185 This regexp still needs some tweaking, right now if i pass holahttps://okular.org; it returns https://okular.org that in my opinion is not correct. You should enforce spaces (or newlines or tabs) around the regexp to make sure the url is a single word. Also it would be cool if you could write down all the urls you've used to positively and negatively test the regexp so we can put it in a testcase Jaydeep Solanki wrote: You mean holahttps://okular.org; should not be detected as an url, hola https://okular.org; should be. Am I right ?? Albert Astals Cid wrote: Exactly Try to have a look on how konversation extracts url from strings. More precisely the function extractUrlData in common.cpp - Mailson --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review27846 --- On Feb. 20, 2013, 2:21 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 20, 2013, 2:21 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 20, 2013, 2:21 p.m.) Review request for Okular. Changes --- icon removed. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On Jan. 27, 2013, 7:02 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2775 http://git.reviewboard.kde.org/r/108614/diff/1/?file=109368#file109368line2775 Why did you decide that icon name? Jaydeep Solanki wrote: I had two options for goto icon, go_goto go_goto_page but none of them shows an icon. I decided to keep, the later one because I have used it in one of my apps, at that time it worked. Albert Astals Cid wrote: Use kdialog --geticon foo, then select again the Actions combo (seems that's a bug) and once you are there try to look for an icon that makes sense, if not, i guess it's better to simply use no icon than one that doesn't exist Jaydeep Solanki wrote: Don't you think that the 'search for' option that appears when right clicked on selected text, should have 'edit-web-search' icon, go to 'link' should have 'preferences-web-browser-shortcuts'. I just had a look at the icons I think 'edit-web-search' describes the search action more appropriately. I agree these icons look good in the current theme I am using but the names seem to say they are though for other actions, meaning in other theme or in the future they may chage and not be useful for us at all. I think i'd go with no icons for now. - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review26271 --- On Feb. 17, 2013, 7:04 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 17, 2013, 7:04 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review27674 --- ui/pageview.cpp http://git.reviewboard.kde.org/r/108614/#comment20726 Please do not return 0 as a QString, that's just evil ui/pageview.cpp http://git.reviewboard.kde.org/r/108614/#comment20727 isEmpty is bigger than isNull, so just check for empty is enough - Albert Astals Cid On Feb. 17, 2013, 7:04 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 17, 2013, 7:04 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On Jan. 27, 2013, 7:02 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2775 http://git.reviewboard.kde.org/r/108614/diff/1/?file=109368#file109368line2775 Why did you decide that icon name? Jaydeep Solanki wrote: I had two options for goto icon, go_goto go_goto_page but none of them shows an icon. I decided to keep, the later one because I have used it in one of my apps, at that time it worked. Use kdialog --geticon foo, then select again the Actions combo (seems that's a bug) and once you are there try to look for an icon that makes sense, if not, i guess it's better to simply use no icon than one that doesn't exist - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review26271 --- On Feb. 17, 2013, 7:04 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 17, 2013, 7:04 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
On Jan. 27, 2013, 7:02 p.m., Albert Astals Cid wrote: ui/pageview.cpp, line 2775 http://git.reviewboard.kde.org/r/108614/diff/1/?file=109368#file109368line2775 Why did you decide that icon name? Jaydeep Solanki wrote: I had two options for goto icon, go_goto go_goto_page but none of them shows an icon. I decided to keep, the later one because I have used it in one of my apps, at that time it worked. Albert Astals Cid wrote: Use kdialog --geticon foo, then select again the Actions combo (seems that's a bug) and once you are there try to look for an icon that makes sense, if not, i guess it's better to simply use no icon than one that doesn't exist Don't you think that the 'search for' option that appears when right clicked on selected text, should have 'edit-web-search' icon, go to 'link' should have 'preferences-web-browser-shortcuts'. I just had a look at the icons I think 'edit-web-search' describes the search action more appropriately. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review26271 --- On Feb. 17, 2013, 7:04 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 17, 2013, 7:04 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Feb. 17, 2013, 7:04 p.m.) Review request for Okular. Changes --- fixed all, but the icon problem. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs (updated) - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 108614: Open url in browser
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/#review26271 --- ui/pageview.cpp http://git.reviewboard.kde.org/r/108614/#comment20004 Would this detect hellohttp://foo.comlala As a link? Also did you create the regex yourself or it comes from somewhere? ui/pageview.cpp http://git.reviewboard.kde.org/r/108614/#comment20001 declare the url as const ui/pageview.cpp http://git.reviewboard.kde.org/r/108614/#comment20002 comparing a QString against 0 is bad ui/pageview.cpp http://git.reviewboard.kde.org/r/108614/#comment20003 Why did you decide that icon name? ui/pageview.cpp http://git.reviewboard.kde.org/r/108614/#comment20005 I'd prefer if you used new KRun(url) here, it's supposed to be more KDE integrated - Albert Astals Cid On Jan. 27, 2013, 2:25 p.m., Jaydeep Solanki wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108614/ --- (Updated Jan. 27, 2013, 2:25 p.m.) Review request for Okular. Description --- When selected text is right clicked, it checks for a url, if found, a QAction is added to open the url in the default browser. This addresses bug 281027. http://bugs.kde.org/show_bug.cgi?id=281027 Diffs - ui/pageview.cpp 60a273d Diff: http://git.reviewboard.kde.org/r/108614/diff/ Testing --- just check if the icon appears properly, because I have an issue with the icon. Thanks, Jaydeep Solanki ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
