emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: emacsclient's option decoding code


From: Stefan Monnier
Subject: Re: emacsclient's option decoding code
Date: Sat, 01 Nov 2008 22:13:26 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux)

> Please take a look at this fragment of emacsclient.c, which deals with
> various combinations of command-line options:

>   if (!current_frame && !tty && !display)
>     display = egetenv ("DISPLAY");

>   if (display && strlen (display) == 0)
>     display = NULL;

>   if (!tty && display)
>     window_system = 1;
>   else if (!current_frame)
>     tty = 1;

>   /* --no-wait implies --current-frame on ttys when there are file
>        arguments or expressions given.  */
>   if (nowait && tty && argc - optind > 0)
>     current_frame = 1;

>   if (current_frame)
>     {
>       tty = 0;
>       window_system = 0;
>     }

>   if (tty)
>     window_system = 0;

> I find this code highly confusing to the degree of being
> unmaintainable.  Look, for example, how it sets `tty' to zero or
> non-zero, and how this is interleaved with testing whether it is zero
> or non-zero, completely obfuscating the semantics of the code and the
> meaning of each variable.

> I think this should be rewritten, but since I cannot make heads or
> tails out of it, I cannot do that myself without risking to break the
> code.  Please someone make this code speak to a programmer.

Yes, I've written my fair share of it, and I'm not proud of it.
If someone can make it more clear, that'd be welcome.  A marginal
improvement is to add an `else' before the last `if'.

Basically, the first 2 `if's setup the `display' variable; the
subsequent 3 enable various options (window_system, tty, and
current_frame) implied by the context; and the last 2 choose one among
the 3 mutually-exclusive options `current_frame', `tty', and
`window_system'.


        Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]