Re: Review Request: Remember current desktop when changing activity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review12068 --- This review has been submitted with commit 6397ef5f8977af0d867c7ab72b0ccf8cb9011771 by Lamarque V. Souza to branch KDE/4.8. - Commit Hook On March 18, 2012, 9:22 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 18, 2012, 9:22 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager_p.h d054eb7 service/ActivityManager.cpp 7af2049 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review12069 --- This review has been submitted with commit 43b05d53caa0f382dcf88e6e9ec9d2b9d7c4fdae by Lamarque V. Souza to branch master. - Commit Hook On March 18, 2012, 9:22 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 18, 2012, 9:22 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager_p.h d054eb7 service/ActivityManager.cpp 7af2049 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
On March 21, 2012, 6:29 p.m., Ivan Čukić wrote: Ship It! makis marimpis wrote: please forgive my ignorance, but should i close it as submitted or wait? :-) You should have committed it with the REVIEW keyword. See http://techbase.kde.org/Policies/SVN_Commit_Policy#Special_keywords_in_SVN_log_messages which also applies to Git. - Ingo --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11705 --- On March 18, 2012, 9:22 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 18, 2012, 9:22 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager_p.h d054eb7 service/ActivityManager.cpp 7af2049 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11705 --- Ship it! Ship It! - Ivan Čukić On March 18, 2012, 9:22 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 18, 2012, 9:22 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager_p.h d054eb7 service/ActivityManager.cpp 7af2049 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
On March 21, 2012, 6:29 p.m., Ivan Čukić wrote: Ship It! please forgive my ignorance, but should i close it as submitted or wait? :-) - makis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11705 --- On March 18, 2012, 9:22 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 18, 2012, 9:22 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager_p.h d054eb7 service/ActivityManager.cpp 7af2049 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
In data mercoledì 21 marzo 2012 18:52:45, makis marimpis ha scritto: On March 21, 2012, 6:29 p.m., Ivan Čukić wrote: Ship It! please forgive my ignorance, but should i close it as submitted or wait? :-) You can use REVIEW: X (where is the review ID) in the commit message (on a separate line), and it will be automatically closed. -- Luca Beltrame - KDE Forums team KDE Science supporter GPG key ID: 6E1A4E79 signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
On March 16, 2012, 12:49 p.m., Ivan Čukić wrote: makis marimpis wrote: Hm, i did that in order to restore the desktop ids from a previous run of kamd (let's say, in case of log out). Ivan Čukić wrote: You misunderstood, I don't mind saving it in the config file, I don't understand the need to keep all those in memory. For example, Bob has 20 activities, usually uses only 3 of them. Why would you want to keep the rest of the VD IDs in memory? Just read the VD ID when necessary. As you can see, we are not keeping anything that is saved to a config file in memory apart from the list of activities. The names, icons etc. are read from the config when needed. makis marimpis wrote: Now i see your point. I have implemented the same patch using only KConfigGroup but because of the scheduleConfigSync there is a case where the sync cannot keep up with the change of the activities - leading to a weird behavior (the changes are not yet made volatile). That could be solved by calling configSync explicitly but that could affect the performace? (no idea). To sum up: i think it is faster and less error-prone to store in memory and sync whenever sync is scheduled to, than to read/write whenever an activity is changed. Not knowing the code around this, I would just like to point out that KConfig/KConfigGroup is already a cache in memory. So, reading/writing should be fast. KConfig::sync is the slow bit (reading from disk if it was changed from the outside, then writing to disk). Can't comment on less error-prone though, I don't know the activity code -- but indeed there's the general risk of a partial config read followed by a save all, which loses everything that hadn't been read yet. Happened in old kmail far too many times. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11467 --- On March 16, 2012, 11:55 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 16, 2012, 11:55 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager.cpp 7af2049 service/ActivityManager_p.h d054eb7 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 18, 2012, 9:22 a.m.) Review request for KDE Runtime and Plasma. Changes --- Updated to store/retrieve activity = desktop id only to/from KConfigGroup. Seems to work just fine :-) Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs (updated) - service/ActivityManager_p.h d054eb7 service/ActivityManager.cpp 7af2049 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
On March 16, 2012, 12:49 p.m., Ivan Čukić wrote: makis marimpis wrote: Hm, i did that in order to restore the desktop ids from a previous run of kamd (let's say, in case of log out). You misunderstood, I don't mind saving it in the config file, I don't understand the need to keep all those in memory. For example, Bob has 20 activities, usually uses only 3 of them. Why would you want to keep the rest of the VD IDs in memory? Just read the VD ID when necessary. As you can see, we are not keeping anything that is saved to a config file in memory apart from the list of activities. The names, icons etc. are read from the config when needed. - Ivan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11467 --- On March 16, 2012, 11:55 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 16, 2012, 11:55 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager.cpp 7af2049 service/ActivityManager_p.h d054eb7 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
On March 16, 2012, 12:49 p.m., Ivan Čukić wrote: makis marimpis wrote: Hm, i did that in order to restore the desktop ids from a previous run of kamd (let's say, in case of log out). Ivan Čukić wrote: You misunderstood, I don't mind saving it in the config file, I don't understand the need to keep all those in memory. For example, Bob has 20 activities, usually uses only 3 of them. Why would you want to keep the rest of the VD IDs in memory? Just read the VD ID when necessary. As you can see, we are not keeping anything that is saved to a config file in memory apart from the list of activities. The names, icons etc. are read from the config when needed. Now i see your point. I have implemented the same patch using only KConfigGroup but because of the scheduleConfigSync there is a case where the sync cannot keep up with the change of the activities - leading to a weird behavior (the changes are not yet made volatile). That could be solved by calling configSync explicitly but that could affect the performace? (no idea). To sum up: i think it is faster and less error-prone to store in memory and sync whenever sync is scheduled to, than to read/write whenever an activity is changed. - makis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11467 --- On March 16, 2012, 11:55 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 16, 2012, 11:55 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager.cpp 7af2049 service/ActivityManager_p.h d054eb7 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 16, 2012, 7:40 a.m.) Review request for KDE Runtime and Plasma. Changes --- Added group Plasma to reviewers group as requested. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager.cpp 7af2049 service/ActivityManager_p.h d054eb7 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104261/#review11462
---
generally looks good to me, save for one small issue.
service/ActivityManager.cpp
http://git.reviewboard.kde.org/r/104261/#comment9136
should also check for = 0. also, this statement needs {}s, we use them
even for single line conditionals :)
- Aaron J. Seigo
On March 16, 2012, 7:40 a.m., makis marimpis wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104261/
---
(Updated March 16, 2012, 7:40 a.m.)
Review request for KDE Runtime and Plasma.
Description
---
Patches kactivitymanagerd to store (and restore back) the working current
directory when switching activities.
The activity-changing-behavior is as follows:
1. Say you have two (or more activities) A and B.
2. You are working on activity A on Desktop 4.
3. You switch to activity B (and by default to Desktop 4).
4. Change to Desktop 1.
5. Go back to activity A and (by default) to Desktop 1, while it should move
you to Desktop 4 (this is where my patch kicks in).
I hope it makes sense :-)
This addresses bugs 241864 and 265015.
http://bugs.kde.org/show_bug.cgi?id=241864
http://bugs.kde.org/show_bug.cgi?id=265015
Diffs
-
service/ActivityManager.cpp 7af2049
service/ActivityManager_p.h d054eb7
Diff: http://git.reviewboard.kde.org/r/104261/diff/
Testing
---
Thanks,
makis marimpis
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 16, 2012, 11:55 a.m.) Review request for KDE Runtime and Plasma. Changes --- Update if condition. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs (updated) - service/ActivityManager.cpp 7af2049 service/ActivityManager_p.h d054eb7 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11467 --- service/ActivityManager.cpp http://git.reviewboard.kde.org/r/104261/#comment9139 Why do you want to keep these in memory? - Ivan Čukić On March 16, 2012, 11:55 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 16, 2012, 11:55 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager.cpp 7af2049 service/ActivityManager_p.h d054eb7 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Remember current desktop when changing activity
On March 16, 2012, 12:49 p.m., Ivan Čukić wrote: Hm, i did that in order to restore the desktop ids from a previous run of kamd (let's say, in case of log out). - makis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/#review11467 --- On March 16, 2012, 11:55 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104261/ --- (Updated March 16, 2012, 11:55 a.m.) Review request for KDE Runtime and Plasma. Description --- Patches kactivitymanagerd to store (and restore back) the working current directory when switching activities. The activity-changing-behavior is as follows: 1. Say you have two (or more activities) A and B. 2. You are working on activity A on Desktop 4. 3. You switch to activity B (and by default to Desktop 4). 4. Change to Desktop 1. 5. Go back to activity A and (by default) to Desktop 1, while it should move you to Desktop 4 (this is where my patch kicks in). I hope it makes sense :-) This addresses bugs 241864 and 265015. http://bugs.kde.org/show_bug.cgi?id=241864 http://bugs.kde.org/show_bug.cgi?id=265015 Diffs - service/ActivityManager.cpp 7af2049 service/ActivityManager_p.h d054eb7 Diff: http://git.reviewboard.kde.org/r/104261/diff/ Testing --- Thanks, makis marimpis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
