D14738: Add the markdown entry
Pino Toscano
noreply at phabricator.kde.org
Tue Aug 14 17:52:52 BST 2018
pino added a comment.
Nitpick: please provide a consistent indentation in the new files -- they seem to mix spaces and tabs.
Also, was this tested when cantor is built without the Discount library? How does it behave when trying to add/edit markdown entries, and trying to load worksheets with such entries?
INLINE COMMENTS
> CMakeLists.txt:103
> + KF5::KIOCore KF5::KIOFileWidgets KF5::KIOWidgets
> + Qt5::PrintSupport cantorlibs cantor_config)
> if(LIBSPECTRE_FOUND)
Extra whitespace change.
> CMakeLists.txt:108
> +if(Discount_FOUND)
> + target_link_libraries(cantorpart ${Discount_LIBRARIES})
> +endif(Discount_FOUND)
FindDiscount.cmake creates a `Discount::Lib` imported library that you can use, and it's even better to do so: it makes sure the target has both the right include directories and the right libraries needed.
> cantor_part.rc:2
> <!DOCTYPE kpartgui SYSTEM "kpartgui.dtd">
> <kpartgui name="cantor_part" version="2">
> <MenuBar>
Since new actions are added here, you must bump the version number of the ui .rc file, otherwise users with a customized menubar/toolbar will not see any change.
> markdownentry.cpp:31
> +
> +#include <QDebug>
> +
This can be removed, once the extra debug is removed too.
> markdownentry.cpp:56
> + m_textItem->setPlainText(plain);
> + qDebug() << plain;
> +}
Extra debug, no need for it.
> markdownentry.cpp:63
> +
> + qDebug() << plain;
> + QDomElement el = doc.createElement(QLatin1String("Markdown"));
Ditto.
> markdownentry.cpp:78
> + QByteArray mdCharArray = plain.toUtf8();
> + MMIOT* mdHandle = mkd_string(mdCharArray.data(), mdCharArray.size()+1, 0); // get the size of the string in byte
> + if(!mkd_compile(mdHandle, MKD_NOSUPERSCRIPT | MKD_FENCEDCODE | MKD_GITHUBTAGS))
This is leaked in all the cases (i.e. when `mkd_compile` succeedes or not). You need to call `mkd_cleanup()`.
REPOSITORY
R55 Cantor
REVISION DETAIL
https://phabricator.kde.org/D14738
To: kqwyf, pino, #cantor, filipesaraiva
Cc: sirgienko, filipesaraiva, pino, asemke, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180814/779de7f8/attachment.html>
More information about the kde-edu
mailing list