ratpoison-devel
[Top][All Lists]
Advanced

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

Re: [RP] select patch


From: Jeff Abrahamson
Subject: Re: [RP] select patch
Date: Thu, 19 Jun 2014 15:18:59 +0100

Hi, Johannes.

First on the concept side, after thinking overnight about your proposal, I'm less convinced it's not to my taste and more convinced that it's just not what I'm used to in ratpoison.  Indeed, I think if I were designing rp from scratch, I might do precisely what you propose.  Perhaps some others will chime in with thoughts.

In the end, I rarely have same-named windows, so I'm not too invested either way.  ;-)

From a user interface standpoint, I agree with you that a variable probably makes more sense if we want to preserve both behaviors.  Let's see if anyone else has an opinion on preserving the old behavior.  (We might only find out by deprecation, of course. ;-)

I see that there are 62 instances of strcmp() and only 9 instances of strncmp().  :(
Nonetheless (again, others please opine) I'd vote for a strncmp() with a new constant in, say, conf.h, rather than saying strcmp(-, -, 40).
Anyway, setting the length at the top and then using strncmp() lets the rest of the function run without needing two blocks, no?  Although, as Bernhard notes, it's unclear why one is case-sensitive and the other not.

int max_match_length;
if(exact_match)
  {
    max_match_length = MAX_WINDOW_NAME_LENGTH;
  }
else
  {
    max_match_length =  strnlen(name, MAX_WINDOW_NAME_LENGTH);
  }


Jeff Abrahamson
+33 6 24 40 01 57   <-- brièvement indisponible le 4 juillet
+44 7920 594 255    <-- will change 18 July

http://jeff.purple.com/
http://blog.purple.com/jeff/



On 19 June 2014 15:09, Bernhard R. Link <address@hidden> wrote:

Looking at the code I realized some things (independent of the patch):

- the only other user of find_window_name is the arg_WINDOW handling.
  I cannot find anything using that.
  Perhaps it makes sense to either make select using that (which likely
  means also telling the arg parsing code that arg_WINDOW is an
  arg_REST/arg_COMMAND/arg_SELLCMD/arg_RAW like argument consuming all
  remaining arguments) or just to remove the whole dead
  read_window/arg_WINDOW code. (which would also make the patch much
  easier, as it could just change the function instead of duplicating it).

- find_window_name defines an exact match to be case-sensitive and
  matching the full string, while in inexact match is case-insensitive
  and a prefix. I wonder if it makes sense to split that and looking
  for cense-sensitive prefixes first and only then case-insensive
  matches.


        Bernhard R. Link
--
F8AC 04D5 0B9B 064B 3383  C3DA AFFC 96D1 151D FFDC

_______________________________________________
Ratpoison-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/ratpoison-devel


reply via email to

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