Re^2: Regex help/ Lua parse

[marquezc329]

I hope that it is not in bad taste to reply twice to your response. If so please forgive me.

After reading your response I read all the linked material and reviewed the code that you provided. Once I was sure I understood your code, I modified my own. Then, taking your comments into consideration, I wrote a new (similar) piece to parse Awesome's theme configuration file. I tried to avoid capturing any data that wouldn't be subsequently referred to. I made sure to read the file only once, and instead assigned the info to related hashes of the form:

 variable_name => value

I provided my code below, first to let you know that the time spent on your response was not in vain, and also in hopes that you would let me know of any complicated/unreadable methods, and/or style/convention infractions before I move on to adding some more interesting functionality. This is some of the most interesting and difficult code I've written thus far in my learning and I plan on expanding it, but I want to keep it clean.

I've been nose deep in Friedl's Mastering Regular Expressions, and it's proven to be extremely useful. I've found regexps to be easier and more powerful, for my purposes, than the Modules that I referred to above. As a result, the regexps in this code are rather long, but I hope still readable.

I've provided my code followed by a larger chunk of the awesome configuration file below:

---

#!/usr/bin/perl

use strict;
use warnings;

use Data::Dumper;

my %theme_table;
my %taglist_table;
my %menu_table;

sub buildTables {
    while (<>) { 
        if (/^\s*(theme\.(?:bg|fg|border|font)(?:\_(?:normal|focus|urg
+ent|minimize|marked|width))?)\s*=\s*\"([^"]*)\"\s*$/)
        { $theme_table{$1} = $2 }
        if (/^\s*(theme\.taglist\_(?:bg|fg)\_(?:normal|focus|urgent))\
+s*=\s*\"([^"]*)\"\s*$/)
        { $taglist_table{$1} = $2 }
        if (/^\s*(theme\.menu\_(?:bg|fg|border|height|width)(?:\_(?:no
+rmal|focus|color|width))?)\s*=\s*\"([^"]*)\"\s*$/)
        { $menu_table{$1} = $2 }
        }
        #print Dumper \%theme_table;
        #print Dumper \%taglist_table;
        #print Dumper \%menu_table;
}

sub printSettings {
    print "=" x 32, "\n";
    print " AWESOME CONFIGURATION SETTINGS \n";
    print "=" x 32, "\n\n";

    foreach my $table (@_) { 
        foreach my $key (sort keys %$table) { print "    $key => $tabl
+e->{$key}\n" }  
        print "\n";
    }
}

buildTables;
printSettings(\%theme_table, \%taglist_table, \%menu_table);

---

---------------------------
-- Default awesome theme --
---------------------------

theme = {}

theme.font          = "sans-serif 8: bold"

--default bg_normal = #222222
theme.bg_normal     = "#000000"
--default bg_focus = #535d6c
theme.bg_focus      = "#000000"
theme.bg_urgent     = "#ffffff"
theme.bg_minimize   = "#ff0000"

--default fg_normal = #aaaaaa
theme.fg_normal     = "#ffffff"
--default fg_focus = #ffffff
theme.fg_focus      = "#55B043"
theme.fg_urgent     = "#ffffff"
theme.fg_minimize   = "#ffffff"

theme.border_width  = "2"
theme.border_normal = "#000000"
--default border_focus = #535d6c
theme.border_focus  = "#55B043"
theme.border_marked = "#91231c"

-- There are other variable sets
-- overriding the default one when
-- defined, the sets are:
-- [taglist|tasklist]_[bg|fg]_[focus|urgent]
-- titlebar_[bg|fg]_[normal|focus]
-- tooltip_[font|opacity|fg_color|bg_color|border_width|border_color]
-- mouse_finder_[color|timeout|animate_timeout|radius|factor]
-- Example:
--theme.taglist_bg_focus = "#ff0000"

theme.taglist_bg_normal = "#000000"
theme.taglist_bg_focus = "#000000"
theme.taglist_fg_normal = "#000000"
theme.taglist_fg_focus = "#55B043"


-- Display the taglist squares
theme.taglist_squares_sel   = "/usr/share/awesome/themes/default/tagli
+st/squarefw.png"
theme.taglist_squares_unsel = "/usr/share/awesome/themes/default/tagli
+st/squarew.png"

theme.tasklist_floating_icon = "/usr/share/awesome/themes/default/task
+list/floatingw.png"

-- Variables set for theming the menu:
-- menu_[bg|fg]_[normal|focus]
-- menu_[border_color|border_width]
theme.menu_bg_normal = "#000000"
theme.menu_bg_focus = "#55B043"
theme.menu_fg_normal = "#55B043"
theme.menu_fg_focus = "#000000"
theme.menu_border_color = "#000000"
theme.menu_border_width = "5"

-- theme.menu_submenu_icon = "/usr/share/awesome/themes/default/submen
+u.png"
theme.menu_height = "15"
-- default menu_width = 100
theme.menu_width  = "150"

---

Thank you, and again, I apologize if this response is in bad taste.

[kcott]

Firstly, you've done nothing wrong in replying twice to the same node. You've done this in a perfectly acceptable fashion: asking different questions which stem from the same previous response. Here's an example of me doing the exactly same thing in last 24 hours: Perl/TK borderwidth question - note the two Re^3: Perl/TK borderwidth question responses. Updating your node (and perhaps sending a /msg indicating such updating) is the more usual way; however, in this instance, you've done the right thing: thunderbolts from the gods may prove me wrong. *gulp* :-)

My personal preference for links to books is that they target the publisher not some arbitrary vendor. A vendor will not advertise books that they do not have in stock. Mastering Regular Expressions is published by O'Reilley and this company, in particular Tim O'Reilley, has been a particularly good friend to Perl over the years - the company your posted link refers to shows no such affiliation. The link I would have provided for this book is: Mastering Regular Expressions (i.e. actual markup: [http://shop.oreilly.com/product/9781565922570.do|Mastering Regular Expressions] [I do have a copy of that book myself and - yes - it's wonderful! :-)]

OK - Off the soap box and back to the code.

Wherever you have non-capturing parentheses containing alternation (e.g (?:x|y|z)), it's usually better to avoid backtracking with the (?>...) construct (e.g. (?>x|y|z)). See perlre - Backtracking, perlre - Extended Patterns and pp. 102-107 in Friedl's book (check the index - page numbers may differ in your version).

You say: "As a result, the regexps in this code are rather long, but I hope still readable.". There's absolutely no reason for them to be unreadable due to length: just use the x option. Furthermore, for those variables not dependent on a loop variable, you can compile them outside of the loop once. Here's an example:

sub some_function {
    my $re = qr{
        \A               # start of string
        (?> x | y | z )  # match exactly one of x, y or z
        \z               # end of string (ignore optional terminal new
+line)
    }x;

    while (<>) {
        if (/$re/} {
            # do something based on successful match
        }
    }
}

Finally, I am absolutely not going to tell you to adopt any particular coding style; however, I am going to urge you to adopt a coding style that's easy for you and others to read. Have a look around the Monastery, see how other Monks write their code, then pick something you're comfortable with. Beyong the indentation issues, the code you presented at the start of this thread was superior to what you now have. If you seriously don't understand what you read in perlstyle, then please ask for clarification.

[marquezc329]

Thank you for your response. Updated the reference to Mastering Regular Expressions above. Can you please explain how the previous code was superior? I see the indentation problems that you are referring to, and I tried to be sure to follow the guidelines specified in perlstyle, i.e. 4 space indentations, curly brackets on same line or lined up vertically, space around most operators. Should I have used

 $theme_table{$1} = $2 if (/regex/); instead of  if (/regex/) { $theme_table{$1}

or

foreach my $table (@_) {
    foreach my $key (sort keys %$table) {
        print "$key => $table->{$key}\n"
    }
    print "\n";
}

instead of

foreach my $table (@_) { 
        foreach my $key (sort keys %$table) { print "    $key => $tabl
+e->{$key}\n" }  
        print "\n";
}

If you could clarify exactly where it is that I'm missing the mark stylistically it would be greatly appreciated. I think good style is the most difficult thing to learn out of a book. Thanks again for your help.

[kcott]

Compare your original version (with all regexes reduced to regex to alleviate noise)

sub buildTables {
    while (<>) { 
        if (/regex/)
        { $theme_table{$1} = $2 }
        if (/regex/)
        { $taglist_table{$1} = $2 }
        if (/regex/)
        { $menu_table{$1} = $2 }
        }
        #print Dumper \%theme_table;
        #print Dumper \%taglist_table;
        #print Dumper \%menu_table;
}

with this alternative version

sub buildTables {
    while (<>) {
        if (/regex/) {
            $theme_table{$1} = $2
        }   
        if (/regex/) {
            $taglist_table{$1} = $2
        }   
        if (/regex/) {
            $menu_table{$1} = $2
        }   
    }
}

Which looks clearer to you? Is the closing brace of the while loop more or less obvious to you now? How about providing me with some examples of code that looks like yours.