HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Autodetecting monitors in XFCE

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
monitorsautodetectingxfce

Problem

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 [[, ], ...]
  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 [[, ] ...] 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?

Solution

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_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 [[, ] ...] 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 [[, ] ...] 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

Code Snippets

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
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
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

Context

StackExchange Code Review Q#597, answer score: 3

Revisions (0)

No revisions yet.