Merge lp:~cqql/lightdm/cpp into lp:lightdm

Proposed by Marten Lienen on 2015-04-26
Status: Merged
Merged at revision: 2146
Proposed branch: lp:~cqql/lightdm/cpp
Merge into: lp:lightdm
Diff against target: 22 lines (+3/-3)
1 file modified
debian/lightdm-session (+3/-3)
To merge this branch: bzr merge lp:~cqql/lightdm/cpp
Reviewer Review Type Date Requested Status
Robert Ancell 2015-04-26 Approve on 2015-05-26
iGEL (community) Approve on 2015-05-25
Review via email: mp+257486@code.launchpad.net

Commit message

Enable the C preprocessor when loading X resources

A common use for X resource files is customization of a user's terminal. Widely
used color schemes use the C preprocessor to avoid repetition. See for example
solarized[1] or base-16[2].

The counter argument, that -nocpp leads to a performance improvement for the
general audience, was disproved by igel and roelof000 in lp:1244578. To
summarize, the speed improvement is in the range of 0.01 seconds and the
commands will not even be executed if you do not have X resource files.

[1] https://github.com/altercation/solarized/blob/master/xresources/solarized
[2] https://github.com/chriskempson/base16-xresources

Description of the change

Enable the C preprocessor when loading X resources

A common use for X resource files is customization of a user's terminal. Widely
used color schemes use the C preprocessor to avoid repetition. See for example
solarized[1] or base-16[2].

The counter argument, that -nocpp leads to a performance improvement for the
general audience, was disproved by igel and roelof000 in lp:1244578. To
summarize, the speed improvement is in the range of 0.01 seconds and the
commands will not even be executed if you do not have X resource files.

[1] https://github.com/altercation/solarized/blob/master/xresources/solarized
[2] https://github.com/chriskempson/base16-xresources

To post a comment you must log in.
lp:~cqql/lightdm/cpp updated on 2015-04-26
2140. By Marten Lienen on 2015-04-27

Enable the C preprocessor when loading X resources

A common use for X resource files is customization of a user's terminal. Widely
used color schemes use the C preprocessor to avoid repetition. See for example
solarized[1] or base-16[2].

The counter argument, that -nocpp leads to a performance improvement for the
general audience, was disproved by igel and roelof000 in lp:1244578. To
summarize, the speed improvement is in the range of 0.01 seconds and the
commands will not even be executed if you do not have X resource files.

[1] https://github.com/altercation/solarized/blob/master/xresources/solarized
[2] https://github.com/chriskempson/base16-xresources

iGEL (igel) wrote :

Thanks Marten!

Dear LightDM Team, could we get a review for these 3 lines here?

Regards, Johannes

review: Approve
Robert Ancell (robert-ancell) wrote :

This code was copied from GDM so the exact reasoning for the nocpp option is not completely clear (I assume performance). Given the number of people wanting it and the lack of strong evidence that it has a significant performance hit lets enable it and see if anyone complains.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/lightdm-session'
2--- debian/lightdm-session 2014-11-24 19:52:08 +0000
3+++ debian/lightdm-session 2015-04-26 23:49:27 +0000
4@@ -47,15 +47,15 @@
5 if [ -d "$xresourcedir" ]; then
6 for file in $xresourcedir/*; do
7 echo "Loading resource: $file"
8- xrdb -nocpp -merge "$file"
9+ xrdb -merge "$file"
10 done
11 fi
12 xresourcefile="$HOME/.Xresources"
13 if [ -f "$xresourcefile" ]; then
14 echo "Loading resource: $xresourcefile"
15- xrdb -nocpp -merge "$xresourcefile"
16+ xrdb -merge "$xresourcefile"
17 fi
18-fi
19+fi
20
21 # Load keymaps
22 if type setxkbmap >/dev/null 2>&1; then

Subscribers

People subscribed via source and target branches