> On April 1, 2013, 4:48 p.m., Thorsten Zachmann wrote: > > libs/kotext/KoTextEditor.cpp, line 1448 > > <http://git.reviewboard.kde.org/r/109746/diff/7/?file=126911#file126911line1448> > > > > Would it be possible to use a static QRegExp or a membervariable here > > as regenerating a QRegExp all the time is quite expensive.
Fixed this as well as a similar code in LinkInsertionDialog.cpp > On April 1, 2013, 4:48 p.m., Thorsten Zachmann wrote: > > plugins/textshape/ReferencesTool.h, line 93 > > <http://git.reviewboard.kde.org/r/109746/diff/7/?file=126913#file126913line93> > > > > can this method made const? I don't think so because we call non const functions in it. > On April 1, 2013, 4:48 p.m., Thorsten Zachmann wrote: > > plugins/textshape/dialogs/LinkInsertionDialog.cpp, line 179 > > <http://git.reviewboard.kde.org/r/109746/diff/7/?file=126916#file126916line179> > > > > please remove the blank before the ) The function was deleted. > On April 1, 2013, 4:48 p.m., Thorsten Zachmann wrote: > > plugins/textshape/dialogs/ManageBookmarkDialog.cpp, line 25 > > <http://git.reviewboard.kde.org/r/109746/diff/7/?file=126920#file126920line25> > > > > Using a static here seems to be wrong to me. Since this is a global variable that won't possibly be used in any other file, adding static is actually a good thing if I am not wrong. (This is not my code) > On April 1, 2013, 4:48 p.m., Thorsten Zachmann wrote: > > plugins/textshape/dialogs/ManageBookmarkDialog.cpp, line 85 > > <http://git.reviewboard.kde.org/r/109746/diff/7/?file=126920#file126920line85> > > > > The test for isNull seem wrong to me as it will break the loop without > > setting a name. However if the nsma is empty it does not. Removing it > > should be enough. Indeed. Since empty input can never be accepted (default behavior) as you have noted in another issue, the isEmpty() check is redundant as well. I have tried to improve it. > On April 1, 2013, 4:48 p.m., Thorsten Zachmann wrote: > > plugins/textshape/dialogs/SimpleLinksWidget.h, line 50 > > <http://git.reviewboard.kde.org/r/109746/diff/7/?file=126921#file126921line50> > > > > I think this should be change to a QList<QString>. This was deleted altogether. --- Thanks for the review. > On April 1, 2013, 4:48 p.m., Thorsten Zachmann wrote: > > plugins/textshape/dialogs/ManageBookmarkDialog.cpp, lines 125-131 > > <http://git.reviewboard.kde.org/r/109746/diff/7/?file=126920#file126920line125> > > > > This allows adding of a bookmark with an empty name. However when > > changing the name a empty name is not allowed which is inconsistent. Fixed by using a similar approach to rename. - Aman ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109746/#review30193 ----------------------------------------------------------- On March 31, 2013, 4:17 a.m., Aman Madaan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109746/ > ----------------------------------------------------------- > > (Updated March 31, 2013, 4:17 a.m.) > > > Review request for Calligra. > > > Description > ------- > > This patch adds the following features : > > 1. Inserting hyperlinks > -- User has a choice of inserting a link by specifying the target and the > link text. Additionally, the > user may supply values for target and rel attributes using a drop down > list. > -- The user may fetch the title from the web page itself (contents of the > "title" tag ). This feature also > handles URL redirects. For example, ieee.com is finally redirected to > http://www.ieee.org/index.html. > This feature will especially help in cases when a user has a list of > links referred to and a list of > references has to be created. Just copy/pasting the URL, clicking fetch > and then insert will do the job. > > 2. Linking to bookmarks > -- A user can specify a bookmark name and the link text. To help the user > with inserting bookmark, > an auto completer is used. This becomes helpful when the bookmarks have > been given a name that > are related to the context. (lastpagefirstpara or conclusions). > > 3. Adding Bookmark using a labeled widget ( similar to the way footnote and > endnote labels are entered ) > > > Diffs > ----- > > libs/kotext/KoTextEditor.h f96ab70 > libs/kotext/KoTextEditor.cpp 07bcad8 > plugins/textshape/CMakeLists.txt b2bf9ae > plugins/textshape/ReferencesTool.h 21caea8 > plugins/textshape/ReferencesTool.cpp 46bf06f > plugins/textshape/dialogs/LinkInsertionDialog.h PRE-CREATION > plugins/textshape/dialogs/LinkInsertionDialog.cpp PRE-CREATION > plugins/textshape/dialogs/LinkInsertionDialog.ui PRE-CREATION > plugins/textshape/dialogs/ManageBookmark.ui PRE-CREATION > plugins/textshape/dialogs/ManageBookmarkDialog.h PRE-CREATION > plugins/textshape/dialogs/ManageBookmarkDialog.cpp PRE-CREATION > plugins/textshape/dialogs/SimpleLinksWidget.h PRE-CREATION > plugins/textshape/dialogs/SimpleLinksWidget.cpp PRE-CREATION > plugins/textshape/dialogs/SimpleLinksWidget.ui PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/109746/diff/ > > > Testing > ------- > > I have tested the following components : > > 1 Inserting Hyperlinks > 1.1 Simplest case (works) > 1.2 Fetch title from URL (works) > 1.3 Fetch title from URL which has re directions (works) > 1.4 specify a URL without a scheme (works, appending an http:// to all the > schemeless urls) > 1.5 fetching from an non existing URL (Time out occurs and notifies user > about the same) > > 2 Adding Bookmarks > 2.1 Simplest case (works) > 2.2 Adding a duplicate bookmark i.e. using a name that has been used > before(User is notified and bookmark addition is aborted) > > 3 Adding link to a bookmark > 3.1 Simplest case (works) > 3.2 Adding a link to a non existing bookmark (works,User is notified about > non existence of the bookmark) > > 4 Manage bookmarks > 4.1 user clicks on manage bookmarks (works, Copy pasted the existing > implementation. Looks like delete bookmark is broken.) > > > Thanks, > > Aman Madaan > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
