help me with sawfish-client bug.

[Teika Kazura]

Hi. Recently, I changed sawfish-client so that you can exit from the
client session with ",quit". (Previously, it killed Sawfish WM.)

But no, it failed. The installed one at /usr/bin/sawfish-client
behaves just like the old one. I confirmed it's the new version (with
'less' :).

But I tried the source in git, scripts/sawfish-client.jl, and it works!
The byte-compiled one also works.

What matters?

Thank you for reading,
Teika (Teika kazura)

[Flashrider]

Am Donnerstag, den 22.10.2009, 17:02 +0900 schrieb Teika Kazura:

> Hi. Recently, I changed sawfish-client so that you can exit from the
> client session with ",quit". (Previously, it killed Sawfish WM.)
>
> But no, it failed. The installed one at /usr/bin/sawfish-client
> behaves just like the old one. I confirmed it's the new version (with
> 'less' :).
>
> But I tried the source in git, scripts/sawfish-client.jl, and it works!
> The byte-compiled one also works.
>
> What matters?
>
> Thank you for reading,
> Teika (Teika kazura)
>

Well it does also work here, just perfect. did you make sure you don't
have another SC in $PATH?

Chris

[Timo Korvola-2]

On Thursday 22 October 2009 11:02:35 Teika Kazura wrote:
> Hi. Recently, I changed sawfish-client so that you can exit from the
> client session with ",quit". (Previously, it killed Sawfish WM.)

Why did you use exception handling for something this trivial?  It would
have been simpler to do this:
(when (equal ",quit\n" input)
  (setq input nil))
and let normal control flow handle it from there as if ^D had been
typed.

> But no, it failed. The installed one at /usr/bin/sawfish-client
> behaves just like the old one.

Works for me as long as one types ",quit" exactly, not any of the
normally valid alternatives such as ", quit" or ",quit ".  A more exotic
failure case is this:
(let ((quit 'lose))
  `(this should not
,quit
))

An alternative approach would be to modify the behaviour of ",quit" in
Sawfish; see the attached patch.  Then no special input handling is
needed in sawfish-client.  Unfortunately there is no way to redefine an
interpreter command locally, hence the patch redefines ,quit globally.

--
        Timo Korvola <URL:http://www.iki.fi/tkorvola>

[sawfish-quit.patch]

---

diff --git a/scripts/sawfish-client.jl b/scripts/sawfish-client.jl
index 4b03cab..fea6d4a 100644
--- a/scripts/sawfish-client.jl
+++ b/scripts/sawfish-client.jl
@@ -100,6 +100,8 @@ sawfish comes with ABSOLUTELY NO WARRANTY; for details see the file COPYING\n"
      (let ((r (sawfish-client-eval
        `(progn
   (require 'rep.util.repl)
+                          (define-repl-command 'quit
+                            (lambda () (throw 'sawfish-client-exit)))
   (make-repl 'user)))))
        (let-fluids ((current-repl r))
  (write standard-output "\nEnter `,help' to list commands.\n")
@@ -107,9 +109,6 @@ sawfish comes with ABSOLUTELY NO WARRANTY; for details see the file COPYING\n"
    (let ((input (readline
  (format nil (if (repl-pending r) "" "%s> ")
  (repl-struct r)))))
-     (when (equal ",quit\n" input)
-                       (throw 'bye nil)
-                       )
      (when input
        (let ((out (sawfish-client-eval
    `(progn
@@ -118,7 +117,9 @@ sawfish comes with ABSOLUTELY NO WARRANTY; for details see the file COPYING\n"
       (make-string-output-stream))
      (standard-error standard-output)
      (r ',r))
- (cons (and (repl-iterate r ',input) r)
+ (cons (catch 'sawfish-client-exit
+                                                (and (repl-iterate r ',input)
+                                                     r))
       (get-output-stream-string
        standard-output)))))))
  (setq r (car out))
@@ -147,9 +148,7 @@ sawfish comes with ABSOLUTELY NO WARRANTY; for details see the file COPYING\n"
   (sawfish-client-eval
    `(repl-completions ',(fluid current-repl) ',w)))
 
-(catch 'bye
-  (main)
-  )
+(main)
 
 (write standard-output "Bye.\n")
 

---

[Teika Kazura]

On Thursday 22 October 2009 11:02:35 Teika Kazura wrote:
> Hi. Recently, I changed sawfish-client so that you can exit from the
> client session with ",quit". (Previously, it killed Sawfish WM.)
> But no, it failed.

Today I tried again, and it worked. This may indicate a queer bug
in librep that we can't confirm. (sigh) Last time I tried at least
twice, so I'm sure that it's not my mistake.

On Sun, 25 Oct 2009 20:20:01 +0300, Timo Korvola wrote:
> Works for me as long as one types ",quit" exactly, not any of the
> normally valid alternatives such as ", quit" or ",quit ".
> [...]
> An alternative approach would be to modify the behaviour of ",quit" in
> Sawfish; see the attached patch.  Then no special input handling is
> needed in sawfish-client.  Unfortunately there is no way to redefine an
> interpreter command locally, hence the patch redefines ,quit globally.

I see, so yours is better. Thanks, I'll push it.
Redefinition must be harmless in practice. The bare /usr/bin/rep
interactive session can be quitted by ",quit", so it must be the
natural one.
# For those who're worried: (quit) continues to kill Sawfish WM. Only
# ",quit" is affected.

> A more exotic failure case is this:
> (let ((quit 'lose))
>   `(this should not
> ,quit
> ))

First time I saw it I thought it was an extremal case, but no. So
it's good that your patch prevent it, too.

> Why did you use exception handling for something this trivial?  It would
> have been simpler to do this:
> (when (equal ",quit\n" input)
>   (setq input nil))
> and let normal control flow handle it from there as if ^D had been
> typed.

It's trivial; I don't know lisp well. Forgive me ;) It's quite a task
for me to understand what's done in sawfish-client.jl. I didn't know
what "let loop" is when I did it. It is documented seperately from
the usual 'let' in librep.info. (I'll send some librep doc patch
*after* 1.6 news rewritement is done.)

Thanks a lot, Timo and Chris,
Teika (Teika kazura)

[Flashrider]

So is it save to merge your quit branch? I already did locally, just
need to commit and push now.

Chris

[Timo Korvola-2]

On Tuesday 27 October 2009 22:00:34 Christopher Roy Bratusek wrote:
> So is it save to merge your quit branch? I already did locally, just
> need to commit and push now.

Yes, branch "quit" contains the discussed changes.

--
        Timo Korvola <URL:http://www.iki.fi/tkorvola>

[Flashrider]

Am Mittwoch, den 28.10.2009, 01:25 +0200 schrieb Timo Korvola:
> On Tuesday 27 October 2009 22:00:34 Christopher Roy Bratusek wrote:
> > So is it save to merge your quit branch? I already did locally, just
> > need to commit and push now.
>
> Yes, branch "quit" contains the discussed changes.
>

Done.