This revision was automatically updated to reflect the committed changes.
Closed by commit R119:8f1043c641a2: KCM/ComponentChooser Treat cases when there
is no app for a usage (authored by meven).
REPOSITORY
R119 Plasma Desktop
CHANGES SINCE LAST UPDATE
meven marked an inline comment as done.
meven added a comment.
Is it fine for other reviewers ?
REPOSITORY
R119 Plasma Desktop
BRANCH
arcpatch-D27395
REVISION DETAIL
https://phabricator.kde.org/D27395
To: meven, bport, ervin, crossi, davidedmundson, #plasma
Cc: anthonyfieroni,
meven added a comment.
Is it fine for other reviewers ?
REPOSITORY
R119 Plasma Desktop
BRANCH
arcpatch-D27395
REVISION DETAIL
https://phabricator.kde.org/D27395
To: meven, bport, ervin, crossi, davidedmundson, #plasma
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n,
meven updated this revision to Diff 75926.
meven added a comment.
Make validLastCurrentIndex const, newline before {
REPOSITORY
R119 Plasma Desktop
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D27395?vs=75920=75926
BRANCH
arcpatch-D27395
REVISION DETAIL
anthonyfieroni added inline comments.
INLINE COMMENTS
> componentchooser.h:51
>
> +int validLastCurrentIndex() {
> +// m_currentIndex == -1 means there are no previously saved value
{ in new line, add const
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
meven updated this revision to Diff 75920.
meven marked 2 inline comments as done.
meven added a comment.
Add a validLastCurrentIndex function, check emailClientService is not null
before using it
REPOSITORY
R119 Plasma Desktop
CHANGES SINCE LAST UPDATE
ervin added inline comments.
INLINE COMMENTS
> componentchooseremail.cpp:155
> KSharedConfig::Ptr profile =
> KSharedConfig::openConfig(QStringLiteral("mimeapps.list"),
> KConfig::NoGlobals, QStandardPaths::GenericConfigLocation);
> -if (profile->isConfigWritable(true) &&
davidedmundson added a comment.
I thought the bug was that we weren't checking the RC from
KService::serviceByStorageId before using it
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D27395
To: meven, bport, ervin, crossi, davidedmundson, #plasma
Cc:
meven added a comment.
How about you @bport ?
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D27395
To: meven, bport, ervin, crossi, davidedmundson, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus,
fbampaloukas, GB_2,
crossi added a comment.
looks ok to me
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D27395
To: meven, bport, ervin, crossi
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham,
meven added inline comments.
INLINE COMMENTS
> meven wrote in componentchooseremail.cpp:154
> I would rather do the opposite the check below is for historical reasons from
> before my refactoring.
I meant to remove the check below (which I did).
REPOSITORY
R119 Plasma Desktop
REVISION
meven added inline comments.
INLINE COMMENTS
> crossi wrote in componentchooseremail.cpp:154
> While you are here, can you had a check on this pointer. I don't understand
> why there is one below but not here.
I would rather do the opposite the check below is for historical reasons from
meven updated this revision to Diff 75695.
meven marked an inline comment as done.
meven added a comment.
Clean a redundant check, ensure that if the user cancels selecting a custom
app, the right entry is selected and hasChanged reflects the state correctly
REPOSITORY
R119 Plasma Desktop
crossi added inline comments.
INLINE COMMENTS
> componentchooseremail.cpp:154
> } else {
> pSettings->setSetting(KEMailSettings::ClientProgram,
> emailClientService->storageId());
> pSettings->setSetting(KEMailSettings::ClientTerminal,
> emailClientService->terminal() ?
meven updated this revision to Diff 75691.
meven added a comment.
standardize how we check if the combobox has a value to save, allow to mark
the value as changed when there was no value selected at the beginning
REPOSITORY
R119 Plasma Desktop
CHANGES SINCE LAST UPDATE
meven added inline comments.
INLINE COMMENTS
> bport wrote in componentchooserbrowser.cpp:107
> add a space between - and 1
>
> By the way this check seems strange to me, can you confirm it still allow to
> save custom browser ?
Once the user has added browsers to the list, they appear as
meven updated this revision to Diff 75679.
meven marked an inline comment as done.
meven added a comment.
Add a space
REPOSITORY
R119 Plasma Desktop
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D27395?vs=75671=75679
BRANCH
arcpatch-D27384
REVISION DETAIL
bport requested changes to this revision.
bport added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> componentchooserbrowser.cpp:107
> {
> +if (currentIndex() == count() -1) {
> +// no browser installed, nor selected
add a space between - and 1
meven created this revision.
meven added reviewers: bport, ervin, crossi.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
meven requested review of this revision.
REVISION SUMMARY
BUG: 417276
TEST PLAN
Have no email client installed.
In kcm componentchooser
-
19 matches
Mail list logo
