compiled and tested on linux
Details
Details
- Reviewers
tbaumgart - Commits
- R471:e780fe155ce5: Fix cmake warning
R471:c11207d0784f: Fix copyright header
R471:7ef38d80aba3: Remove obsolete qrc file
R471:51254eabc798: Fix copyright header
R471:29d7703e35d9: Fix cmake warning
R471:2616e538cc7b: Use AlkValue instead of AlkMoney
R471:9dc46d39195d: Simplify widget initialization
R471:2aebc943290b: Simplify widget initialization
R471:460d5db0674e: Use AlkValue instead of AlkMoney
R471:4dd2aedb6efd: Remove obsolete qrc file
Diff Detail
Diff Detail
- Repository
- R471 Alkimia Library
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 5824 Build 5842: arc lint + arc unit
Comment Actions
Running AlkOnlineQuoteTest fails with a crash.
| src/alkonlinequotesource.cpp | ||
|---|---|---|
| 330 ↗ | (On Diff #45716) | It crashes here, because d->m_profile == nullptr. |
| src/alkonlinequotesprofile.cpp | ||
| 122 ↗ | (On Diff #45716) | This creates an AlkOnlineQuoteSource default entry with no profile. |
| 187 ↗ | (On Diff #45716) | Using this ctor creates an AlkOnlineQuoteSource where d->m_profile is a nullptr. |
Comment Actions
Qt5 variant
- fix crash in alkonlinequotetest
- fix not finding Qt5QuickCompiler
KDE4 variant
- fix missing libraries on compiling onlinequoteseditor
Comment Actions
- Add cmake option for Qt4 builds
- Add WebPriceQuote related class from KMyMoney
- Add command line app 'onlinequoteseditor'
- add todo
- Add application profile group box
- Rewrite AlkOnlineQuoteSource
- Move out AlkFinanceQuoteProcess
- We need to install the generated ui file
- Add AlkOnlineQuotesProfile and AlkOnlineQuotesProfileManager
- Use AlkOnlineQuotesProfile and AlkOnlineQuotesProfileManager in client code
- Use colored status messages
- Add page preview and web inspector
- ui fixup
- preview fixup
- profiles manager fixup
- alkvalue fixup
- preview fixup
- add more profiles
- use wrebkit by default, add '.kio' postfix to use KIO
- add local knsrc
- webview fixup
- fix signal
- Use external QWebView instance
- readd missing vertical spacer ibn profile block
- Refactor mainwindows to use two columns: editor left and browser right
- fix not exiting AlkOnlineQuote event loop
- Use AlkOnlineQuotesProfile::Type camel case enum value 'KMyMoney'
- Fix order of initialization
- Fix profile selection
- Set build default to Qt4 for now
- Load profile only on 'select' button click
- Removed unused slot
- Fix online quote test
- Disable update button on default
- Do no load specific page by default
- Move 'select' button to top of profile view
- Code reformatted using kde-dev-scripts/uncrustify-kf5
- Add UML design
- Add qt5 build support
- Disable show Button for now
- Fix debug area in class AlkOnlineQuote
- In native mode show html code in integrated browser
- Move private implementation of AlkOnlineQuote into Private class
- Make AlkOnlineQuote::Errors non inlined
- Add language box
- crash fix
- onlinequoteseditor: Only show install button if profile supports GHNS
- Add copyright
- Add QDate date format support required by skrooge GHNS files
- Skrooge GHNS files always require to skip html stripping
- Initiate profile on dialog load
- Wrap dynmic values in message with ''
- Enable skrooge profile to be able to check GHNS support
- Limit height of quotes list to left debug output more vertical room
- Add plasma applet example
- Add configuration settings to plasma applet
- Fix crash in case webkit widget is not configured
- Revert launch method to kio by default
- Only search for GHNS updates in case GHNS profile is used
- fixup initial fetching and redraw
- fix warnings
- fix debug messages
- Make header constant unique
- Add profile selection to plasma applet
- Use the same icon as kmymoney for online quote plasma applet
- Refactor to always use a profile
- Fix bug not loading profile in constructor of plasma applet
- Add KF5 support to plasma applet
- Removed unused include
- Add qml test app for alkonlinequote with qt5
- Add qml plugin for AlkOnlineQuote
- Add Qt4 support for qml plugin
- Fix qml plugin test app for Qt4
- Add copy of foreign currency plasma5 plugin as starting point
- Replace online quote fetch with Alkimia
- Fix loading sources from GHNS profile
- Add GHNS upload support
- Read knsrc 'targetDir' attribute from file on creating profile
- Keep applet name in sync with desktop file
- Add missing cmake file
- Move skrooge related knsrc file into alkimia name space
- Do not export AlkOnlineQuotesWidget for now
- Move internal api from AlkOnlineQuotesWidget into related Private class
- Readd changing profile by selecting on profile entry
- Do not export generated file from AlkOnlineQuotesWidget UI
- Clean build cmake fix
- Add support to migrate AlkOnlineSource from KMyMoney storage to GHNS storage and vice versa
- Use redirected knewstuff url to fix not getting new stuff provider.
- Install all header files in alkimia subdir
- Compile fix
- Fix usage of BUILD_QT4=ON in LibAlkimiaConfig.cmake
- Compile fix
- Make showing profile list and upload button optional with class AlkOnlineQuotesWidget constructor
- Do not export privatly used libraries
- Add translation support
- Fix crash in qt5 variant of alkonlinequotestest
- Add missing libraries to Qt4 variant of onlinequoteseditor
- Add missing Qt5 module for finding qt5 quick compiler
- Do not depend test cases on Qt 5.11
Comment Actions
I still have not figured out how the signal/slot stuff works for the private objects, but that is just a matter of time.
| src/alkexception.cpp | ||
|---|---|---|
| 2 ↗ | (On Diff #46196) | Same here |
| src/alkexception.h | ||
| 2 ↗ | (On Diff #46196) | It was me who wrote this. This must be a copy/paste problem |
| src/alkonlinequoteswidget.cpp | ||
| 99 | Why not profilesGroupBox->setVisible(showProfiles); | |
| 101 | Why not profilesGroupBox->setVisible(showUpload); | |
| 263 | Suggestion for code simplicity // the next line replaces all of the setEnabled calls in this method
// (except the very last)
detailsGroupBox->setEnabled(item != nullptr);
m_editURL->clear(); // better and faster than ->setText(QString())
m_editSymbol->clear();
:
:
if(item) {
:
m_editURL->setText( ... );
:
}
m_updateButton->setEnabled(false); | |
| src/alkquotereceiver.h | ||
| 55 ↗ | (On Diff #46672) | Why don't you use AlkValue here and encapsulate a double into a new object? |
Comment Actions
- Remove obsolate qrc file
- Fix cmake warning
- Fix copyright header
- Simplify widget initialization
- Use AlkValue instead of AlkMoney
Comment Actions
The testcase somehow fails:
********* Start testing of AlkOnlineQuoteTest *********
Config: Using QtTest library 5.9.4, Qt 5.9.4 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 7.3.1 20180323 [gcc-7-branch revision 258812])
PASS : AlkOnlineQuoteTest::initTestCase()
QDEBUG : AlkOnlineQuoteTest::testQuoteSources() using profile "alkimia"
QDEBUG : AlkOnlineQuoteTest::testQuoteSources() ("", "Alkimia Currency")
PASS : AlkOnlineQuoteTest::testQuoteSources()
QDEBUG : AlkOnlineQuoteTest::testLaunch() using profile "alkimia"
QDEBUG : AlkOnlineQuoteTest::testLaunch() test::AlkQuoteReceiver::slotStatus( "(Debug) symbol=EUR USD id=EUR USD..." )
QDEBUG : AlkOnlineQuoteTest::testLaunch() using profile "alkimia"
QDEBUG : AlkOnlineQuoteTest::testLaunch() test::AlkQuoteReceiver::slotError( "Source <placeholder>KMyMoney Currency</placeholder> does not exist." )
FAIL! : AlkOnlineQuoteTest::testLaunch() 'quote.launch("EUR USD", "EUR USD", m_profile->quoteSources().first())' returned FALSE. ()
Loc: [/home/thb/devel/alkimia/autotests/alkonlinequotetest.cpp(54)]
PASS : AlkOnlineQuoteTest::cleanupTestCase()
Totals: 3 passed, 1 failed, 0 skipped, 0 blacklisted, 4ms
********* Finished testing of AlkOnlineQuoteTest *********but I don't know if this is related. Is it the right testcase anyway?
| src/alkwebpage.cpp | ||
|---|---|---|
| 2 | Header with license is missing | |
| 24 | I would change this to AlkWebPage::AlkWebPage(QWidget* parent)
: QWebView(parent)
, d(new Private)
{
}and adjust signature in the header file to AlkWebPage(QWidget* parent = nullptr); | |
| 30 | Don't you need a delete d; here? | |
| 55 | I would name the parameter enable | |
| 58 | I would move that into the private class e.g. d->enableWebInspector(enable, page()); and then AlkWebPage::Private::enableWebInspector(bool enable, QWebPage* page)
{
delete inspector;
inspector = nullptr;
if (enable) {
inspector = new QWebInspector();
inspector->setPage(page);
}
}Your solution has a problem if enableWebInspector is called twice in a row with enable true. This would leave a QWebInspector object dangling. | |
| src/alkwebpage.h | ||
| 1 | Header with license is missing | |
Comment Actions
- rebased
- fixed mentioned issue
- made AlkWebPage an internal class because it is used only in the implementation
| src/alkwebpage.cpp | ||
|---|---|---|
| 49 | In case you have the following user code enableWebInspector(true); enableWebInspector(false); the attribute will not be set to false. | |
Comment Actions
- fix setting of QWebPage settings relatd to WebInspector
- rename web inspector enable method to setWebInspectorEnabled() to keep in sync with other getters
- add getter webInspectorEnabled()