Summary: | bashism echo -e in krandrsetup - "Save as default" fails to restore some saved screen resolution settings with dash | ||
---|---|---|---|
Product: | krandr | Reporter: | Philippe Cloutier <chealer> |
Component: | general | Assignee: | Oswald Buddenhagen <ossi> |
Status: | CLOSED FIXED | ||
Severity: | normal | CC: | apodtele, jowenn, l.lunak, mail, myriam, sven.burmeister |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Debian testing | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | 4.6.5 | |
Attachments: | propsed fix |
Description
Philippe Cloutier
2011-06-10 22:40:21 UTC
Oh, forgot to write that dash is the default /bin/sh on Debian, for performance reasons (although the "default shell" is still bash). This is probably the explanation for 261285 and 265710. Hum OK in fact krandrsetup only appeared in 4.6 so #265710 and my problems on KDE 4.4 must rather be instances of #183143. As krandrsetup is a shell script, there is actually a very simple workaround. Assuming you have bash, edit /usr/bin/krandrsetup changing its first line from #!/bin/sh to #!/bin/bash Heiner Steven, who discovered the problem, proposed an actual fix using printf rather than echo in the Debian bug report. I'm forwarding a message from the original reporter about how he found the problem. I think errors from krandrstartup should not be discarded in case it has more bugs. I tried to find the root cause of the problem by reading the "startkde" script and the involved helper scripts and functions, but could not find a problem. Afterwards I changed "startkde" to include debugging output ("set -vx"), which appeared in the file $HOME/.xsession-errors Still the problem was not visible. Only after I removed the stderr-redirection of "krandrstartup" (change . krandrstartup 2>/dev/null to . krandrstartup #2>/dev/null ) I saw the error message. Created attachment 60928 [details]
propsed fix
please test
That kind of inverts the bug. It will work if $krandrrc_display_startupcommands contains a single command, but only then. $ eval "echo a\necho b" anecho b chealer@vinci:/etc/cron.weekly$ you didn't actually test it or read the commit message, huh? I didn't actually test, but did read the commit message, just found it to be wrong. In fact there's an important confusion. StartupCommands can actually contain escapes (for newlines). Mine for example has one: StartupCommands=xrandr --output "LVDS" --pos 0x0 --mode 1366x768 --refresh 60.042\nxrandr --output "CRT1" --pos 0x0 --mode 1920x1080 --refresh 60 The literal format of Heiner's StartupCommands are on the downstream report: StartupCommands="xrandr --output \"VGA-1\" --pos 0x0 --mode 1600x1200 --refresh 75 xrandr --output \"VGA-1\" --primary" As you can see, the format changed. Heiner's string contains a literal newline and is double quoted. Mine is unquoted and contains an encoded newline. The StartupCommands format must have changed, I don't know why nor when. I asked Heiner to test resetting and generating a new StartupCommands, and he confirmed he now has a different format (same as mine). Therefore the "complicated" code from Luboš must be intended. The echo is required for the new format. FWIW, using ";" as separator would probably be simpler than the new format, allowing your patch to work. This is quite confusing, it should be documented somewhere, probably just around the problematic lines. I believe the following structure would work: eval "`echo -e "$krandrrc_display_startupcommands"`" The backticks for command substitution are POSIX. For echo -e, I suggest following Heiner's suggestion. heiner's config file is plain invalid and i have no idea how he got it that way - kconfig would not write that, and i found no indication in krandr's history that it would bypass kconfig at any time. probably some custom scripting or manual manipulation. your config file will work just fine with my patch. just try it. ;) I tested Oswald's patch and it actually works. The reason is that $krandrrc_display_startupcommands contains actual newlines, no escape sequences. I don't know where $krandrrc_display_startupcommands is defined, but as long as it interprets StartupCommands the same way, Oswald's patch is sufficient. *** Bug 248563 has been marked as a duplicate of this bug. *** Git commit 14d16ce823a1bf4325e0ab632a4893f88b88e231 by Oswald Buddenhagen. Committed on 12/06/2011 at 12:47. Pushed by ossi into branch 'KDE/4.6'. remove pointless bash-ism there are no escapes to process here. and as the newlines appear literally in the first place, there is also no reason for using a loop. BUG: 275360 FIXED-IN: 4.6.5 (cherry picked from commit 175afa1e2569eeee204d660628c06928882e034f) M +1 -4 kcontrol/randr/krandrstartup http://commits.kde.org/kde-workspace/14d16ce823a1bf4325e0ab632a4893f88b88e231 Git commit 175afa1e2569eeee204d660628c06928882e034f by Oswald Buddenhagen. Committed on 12/06/2011 at 12:47. Pushed by ossi into branch 'master'. remove pointless bash-ism there are no escapes to process here. and as the newlines appear literally in the first place, there is also no reason for using a loop. BUG: 275360 FIXED-IN: 4.6.5 M +1 -4 kcontrol/randr/krandrstartup http://commits.kde.org/kde-workspace/175afa1e2569eeee204d660628c06928882e034f What actual newlines? This is frigging idiotic. Use ";" like shells do. However: The problem is not newlines - the problem is multiple commands. Where did you read that it is Ok to split --output's? The man page certainly suggests using using all --output's at once. *** Bug 278133 has been marked as a duplicate of this bug. *** |