4
\$\begingroup\$

I got sick of manually xrandering things on my computers (especially since I always just sequence monitors from left to right and set each at the highest resolution) so I wrote this:

#!/usr/bin/ruby

def xrandrPairs (xList)
## Takes a split list of xrandr output and returns [[<display name>, <max-resolution>], ...]
  pairs = [[matchDisplay(xList[0]), matchOption(xList[1])]]
  (2..xList.length-1).to_a.each do |i| # kind of hacky, but I need to reference car and cadr here, so a call to .map won't do it
    if xList[i] =~ /^\S/ 
      pairs.push([matchDisplay(xList[i]), matchOption(xList[i+1])]) 
    end
  end
  pairs
end

def matchDisplay (dispString)
## Matches a display name
  dispString.match(/^([^\s]*)/)[1]
end

def matchOption (optString)
## Matches a resolution string (since they have whitespace preceding them)
  optString.match(/^\s*([^\s]*)/)[1]
end

def xrandrString (xPairs)
## Takes [[<display name>, <max-resolution>] ...] and returns an xrandr command string
  s = "xrandr --output #{xPairs[0][0]} --mode #{xPairs[0][1]}"
  if xPairs.length >= 2
    (1..xPairs.length-1).to_a.each do |i| # same as above
      s += " --output #{xPairs[i][0]} --mode #{xPairs[i][1]} --right-of #{xPairs[i-1][0]}"
    end
  end
  s
end

exec xrandrString(xrandrPairs(`xrandr`.split("\n")[1..-1]))

The key is that each computer I use has different displays (they're named differently and they have different maximum resolutions), so as far as I know, I have to either parse xrandr output or write a different script for each machine.

I don't care that it's inefficient (traversing the xrandr output multiple times and doing some looped string formatting) because it only runs once at startup time, and deals with a list of 30 elements at the outside. I'm using Ruby 1.8.7 straight out of the Squeeze repos for ease of installation (this is also why I wouldn't mind being shown how this works in Python/Perl; those come with the system).

Can I get some comments on it?

\$\endgroup\$
2
  • \$\begingroup\$ Could you mention your ruby version? \$\endgroup\$
    – sepp2k
    Feb 4, 2011 at 3:46
  • \$\begingroup\$ @sepp2k - mentioned; 1.8.7 \$\endgroup\$
    – Inaimathi
    Feb 4, 2011 at 3:54

1 Answer 1

3
\$\begingroup\$

First of all, the convention in ruby is to use snake_case, not camelCase for variable and method names. It's generally a good idea to adhere to a language's naming conventions - if only so that the code looks consistent when you're calling standard library methods as well as your own.


In your xrandrPairs method, you mention that you're using an index to iterate because you have to go through the array in pairs. You can avoid this by using each_cons(2) which will yield each item together with the item after it (e.g. [1,2,3].each_cons(2) will yield 1,2 in the first iteration and 2,3 in the second).

However there's a better way to do this then to iterate through the lines. You can use scan to find all the lines that start without spaces and extract the information you want in one go:

def xrandr_pairs (xrandr_output)
## Returns [[<display name>, <max-resolution>] ...]
  display_re = /^(\S+)/
  option_re = /^\s+(\S+)/
  xrandr_output.scan(/#{display_re}.*\n#{option_re}/)
end

Since scan returns an array containing one subarray per match where each item in the subarray corresponds to one capturing group in the regex, this will produce the output you want. Note that xrandr_pairs now takes xrandr's output as a string, not an array of lines.

In addition to using scan I also changed the regexen a bit: I replaced [^\s] with \S, which is equivalent but shorter, and used + instead of *, so it does not match empty strings.


The xrandr_string method can also be rewritten to be much nicer by using the each_cons method like this:

def xrandr_string (x_pairs)
## Takes [[<display name>, <max-resolution>] ...] and returns an xrandr command string
  s = "xrandr --output #{x_pairs[0][0]} --mode #{x_pairs[0][1]}"
  x_pairs.each_cons(2) do |(previous_output, previous_mode), (output, mode)|
      s += " --output #{output} --mode #{mode} --right-of #{previous_output}"
    end
  end
  s
end

You don't need to check that the size is at least 2 because each_cons simply doesn't do anything if the array is smaller than the given chunk-size.

I also used the destructuring bind of block arguments to assign the elements of the subarrays to variables directly.

Instead of building up the string imperatively you could also use map and join like this:

def xrandr_string (x_pairs)
## Takes [[<display name>, <max-resolution>] ...] and returns an xrandr command string
  cmd = "xrandr --output #{x_pairs[0][0]} --mode #{x_pairs[0][1]}"
  args = x_pairs.each_cons(2).map do |(previous_output, previous_mode), (output, mode)|
    "--output #{output} --mode #{mode} --right-of #{previous_output}"
  end
  [cmd, *args].join(" ")
end
\$\endgroup\$
5
  • \$\begingroup\$ Sweeeeet. each_cons does precisely what I need here (as you note, it removes the need for any imperative sequence building) and it's much more straightforward to scan for pairs with a single regex. I've noted the snake_case convention for the future; it wasn't an intentional infraction. \$\endgroup\$
    – Inaimathi
    Feb 4, 2011 at 4:33
  • \$\begingroup\$ Some notes though: you need do preceding the |...| sections, option_re should be /^\s+... instead of /^\s*... (this is because xrandr output starts with a non-whitespace-padded line that doesn't reference a monitor; using * will match that as the first monitor, returning a pair that looks something like ["Screen", "DVI-1"]) and you have an extra end in the functional version of xrandr_string. \$\endgroup\$
    – Inaimathi
    Feb 4, 2011 at 4:34
  • \$\begingroup\$ @Inaimathi: Bah, sorry about that. I wrote the code "blind" without testing it and it's a bit late here. Good job reviewing my code though ;-) \$\endgroup\$
    – sepp2k
    Feb 4, 2011 at 4:38
  • \$\begingroup\$ Out of curiosity, would it be more idiomatic to make xrandr_pairs a method of String and xrandr_string a method of Array (and have them both reference self instead of taking an argument) so that I could call them as x_out.xrandr_pairs.xrandr_string instead of xrandr_string(xrandr_pairs(x_out))? \$\endgroup\$
    – Inaimathi
    Feb 5, 2011 at 4:59
  • 1
    \$\begingroup\$ @Inaimathi: No, they're too specialized for that. For the vast majority of strings having a method xrandr_pairs makes no sense. What you could do is to define a class XrandrResult with a pairs method and a class XrandrPairs with a command_string method and then do something like XrandrResult.new(xrandr).pairs.command_string. However at this point that'd just be OO for OO's sake. If you ever want to add additional functionality those classes might be a useful abstraction though. \$\endgroup\$
    – sepp2k
    Feb 5, 2011 at 5:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.