[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 125256: kded_keyboard: Fix restoring application/window specific layout settings
From: "David Rosca" <nowrep () gmail ! com>
Date: 2015-09-16 18:39:16
Message-ID: 20150916183916.2235.9623 () mimi ! kde ! org
[Download RAW message or body]
--===============1217706713980538587==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On Sept. 16, 2015, 1:11 p.m., David Edmundson wrote:
> > +1
> >
> > given it's a behavioural change and I don't use keyboard layout switching (there \
> > is only one true layout!) I'd like someone else to comment on the session \
> > restoring. I can see arguments for both restoring and always starting from the \
> > same layout each time.
>
> David Rosca wrote:
> This only works when you set "Switching Policy" to Desktop, Application or Window \
> and then you kind of expect it to remember the layouts (eg. I set US layout for \
> konsole and CZ for everything else).
> It doesn't affect anything if you have "Switching Policy = Global", which is \
> default.
I'm sorry, you were right. It does actually restore also current layout for global \
layouts.
So either we disable it for global policy (as it wasn't working before), or we can \
fix it. Imho it's better to fix it = no change to this patch.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125256/#review85495
-----------------------------------------------------------
On Sept. 16, 2015, 9:41 a.m., David Rosca wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125256/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 9:41 a.m.)
>
>
> Review request for Plasma.
>
>
> Bugs: 325572
> https://bugs.kde.org/show_bug.cgi?id=325572
>
>
> Repository: plasma-desktop
>
>
> Description
> -------
>
> This removes the check if user has restore session on start option enabled, so the \
> layout map is restored everytime.
> It also checks in LayoutMemory::laoutChanged if the changed keyboard layout is \
> different from user configured to prevent clearing the layout map on startup.
>
> Diffs
> -----
>
> kcms/keyboard/layout_memory.cpp 5ac6729
> kcms/keyboard/layout_memory_persister.cpp da48739
>
> Diff: https://git.reviewboard.kde.org/r/125256/diff/
>
>
> Testing
> -------
>
> Per-application (or per-window) layouts are now preserved.
>
>
> Thanks,
>
> David Rosca
>
>
--===============1217706713980538587==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/125256/">https://git.reviewboard.kde.org/r/125256/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On September 16th, 2015, 1:11 p.m. UTC, <b>David \
Edmundson</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">+1</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">given it's a \
behavioural change and I don't use keyboard layout switching (there is only one true \
layout!) I'd like someone else to comment on the session restoring. I can see \
arguments for both restoring and always starting from the same layout each \
time.</p></pre> </blockquote>
<p>On September 16th, 2015, 1:59 p.m. UTC, <b>David Rosca</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This \
only works when you set "Switching Policy" to Desktop, Application or Window and then \
you kind of expect it to remember the layouts (eg. I set US layout for konsole and CZ \
for everything else).</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">It doesn't affect anything if you have \
"Switching Policy = Global", which is default.</p></pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm \
sorry, you were right. It does actually restore also current layout for global \
layouts.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">So either we disable it for global policy (as it \
wasn't working before), or we can fix it. Imho it's better to fix it = no change to \
this patch.</p></pre> <br />
<p>- David</p>
<br />
<p>On September 16th, 2015, 9:41 a.m. UTC, David Rosca wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma.</div>
<div>By David Rosca.</div>
<p style="color: grey;"><i>Updated Sept. 16, 2015, 9:41 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=325572">325572</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">This removes the check if user has restore session on \
start option enabled, so the layout map is restored everytime.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It \
also checks in LayoutMemory::laoutChanged if the changed keyboard layout is different \
from user configured to prevent clearing the layout map on startup.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Per-application (or per-window) layouts are now \
preserved.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kcms/keyboard/layout_memory.cpp <span style="color: grey">(5ac6729)</span></li>
<li>kcms/keyboard/layout_memory_persister.cpp <span style="color: \
grey">(da48739)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125256/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============1217706713980538587==--
[Attachment #3 (text/plain)]
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic