Hi Bill, 

I received this bug report against the Debian xword package. These
patches look quite useful. Do you have time to consider them for
inclusion in upstream?

I'm CCing Dafydd, who submitted the patches, and
526821-forwar...@bugs.debian.org -- please keep the latter CC in your
reply.

--- Begin Message ---
Package: xword
Version: 1.0-3
Severity: wishlist
Tags: patch

xword assumes that crosswords it reads are American-style, i.e. that any
square that has a black square above it or to its left is at the beginning of
a new solution, which isn't true of British-style crosswords (see
http://en.wikipedia.org/wiki/Crossword#Types_of_grid). Hence it gets very
confused when loading a British-style crossword.

Here is a series of patches that add support for British-style layouts, plus
one fix for keypress handling and and one patch that adds a test suite. These
are taken from my git repository at (http://rhydd.org/git/xword/).
>From fdfb5a210da7f4596323330e72b1fc190611f40e Mon Sep 17 00:00:00 2001
From: Dafydd Harries <d...@rhydd.org>
Date: Sun, 3 May 2009 16:39:40 +0300
Subject: [PATCH 1/6] handle British-style crossword layouts

When allocating numbers to cells, allocate a new number to a cell if the cell:

 - is white;
 - has not already been allocated a number;
 - and it it followed by white cell either to the right or downwards.

When allocating a number, allocate the same number to consecutive white cells
to the right or downwards. Previously, a new number was always allocated if
the cell above or to the left was black.
---
 xword |   61 ++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/xword b/xword
index 1fde0fa..8da2492 100755
--- a/xword
+++ b/xword
@@ -263,35 +263,39 @@ class Puzzle:
         number = 1
         for y in range(self.height):
             for x in range(self.width):
-                is_fresh_x = self.is_black(x-1, y)
-                is_fresh_y = self.is_black(x, y-1)
+                new_across = False
+                new_down = False
 
                 if not self.is_black(x, y):
-                    if is_fresh_x:
-                        self.across_map[x, y] = number
-                        if self.is_black(x+1, y):
-                            self.across_clues[number] = ''
-                        else:
-                            self.across_clues[number] = self.clues.pop(0)
-                    else: self.across_map[x, y] = self.across_map[x-1, y]
+                    if ((x, y) not in self.across_map and
+                            not self.is_black(x+1, y)):
+                        new_across = True
+                        self.across_clues[number] = self.clues.pop(0)
+
+                        for x_ in range(x, self.width):
+                            if self.is_black(x_, y):
+                                break
+
+                            self.across_map[x_, y] = number
                     
-                    if is_fresh_y:
-                        self.down_map[x, y] = number
-                        if self.is_black(x, y+1): # see April 30, 2006 puzzle
-                            self.down_clues[number] = ''
-                        else:
-                            self.down_clues[number] = self.clues.pop(0)
-                    else: self.down_map[x, y] = self.down_map[x, y-1]
-
-                    if is_fresh_x or is_fresh_y:
-                        self.is_across[number] = is_fresh_x
-                        self.is_down[number] = is_fresh_y
+                    if ((x, y) not in self.down_map and
+                            # see April 30, 2006 puzzle
+                            not self.is_black(x, y+1)):
+                        new_down = True
+                        self.down_clues[number] = self.clues.pop(0)
+
+                        for y_ in range(y, self.height):
+                            if self.is_black(x, y_):
+                                break
+
+                            self.down_map[x, y_] = number
+
+                    if new_across or new_down:
+                        self.is_across[number] = new_across
+                        self.is_down[number] = new_down
                         self.number_map[number] = (x, y)
                         self.number_rev_map[x, y] = number
                         number += 1
-                else:
-                    self.across_map[x, y] = 0
-                    self.down_map[x, y] = 0
         self.max_number = number-1
 
     def hashcode(self):
@@ -996,8 +1000,15 @@ class PuzzleController:
                 self.do_update('box-update', xp, yp)
 
             self.do_update('title-update')
-            self.do_update('across-update', self.puzzle.number(x, y, ACROSS))
-            self.do_update('down-update', self.puzzle.number(x, y, DOWN))
+
+            if (x, y) not in self.puzzle.mode_maps[self.mode]:
+                self.switch_mode()
+
+            if (x, y) in self.puzzle.across_map:
+                self.do_update('across-update', self.puzzle.number(x, y, 
ACROSS))
+
+            if (x, y) in self.puzzle.down_map:
+                self.do_update('down-update', self.puzzle.number(x, y, DOWN))
 
     def select_word(self, mode, n):
         if mode <> self.mode: self.switch_mode()
-- 
1.6.2.4

>From 5a2ae0ef3d6c64384863a37df5b371df49c3884f Mon Sep 17 00:00:00 2001
From: Dafydd Harries <d...@rhydd.org>
Date: Sun, 3 May 2009 16:45:38 +0300
Subject: [PATCH 2/6] when switching modes, jump to the first clue of the other 
mode, not clue 1

In US-style crosswords, both across and down modes have a clue 1, but this
assumption breaks in British-style crosswords.
---
 xword |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/xword b/xword
index 8da2492..da69d8c 100755
--- a/xword
+++ b/xword
@@ -382,6 +382,14 @@ class Puzzle:
             if mode == DOWN and self.is_down[n]: break
         return n
 
+    def first_number(self, mode):
+        n = 1
+        while True:
+            if mode == ACROSS and self.is_across[n]: break
+            if mode == DOWN and self.is_down[n]: break
+            n += 1
+        return n
+
     def final_number(self, mode):
         n = self.max_number
         while True:
@@ -1048,7 +1056,7 @@ class PuzzleController:
         n = self.puzzle.incr_number(self.x, self.y, self.mode, incr)
         if n == 0:
             self.switch_mode()
-            if incr == 1: n = 1
+            if incr == 1: n = self.puzzle.first_number(self.mode)
             else: n = self.puzzle.final_number(self.mode)
         (x, y) = self.puzzle.number_map[n]
         (x, y) = self.puzzle.find_blank_cell(x, y, self.mode, 1)
-- 
1.6.2.4

>From 7d9e8dd3b6fef2d2ffbfebc1c512d39639df7bd1 Mon Sep 17 00:00:00 2001
From: Dafydd Harries <d...@rhydd.org>
Date: Sun, 3 May 2009 16:44:15 +0300
Subject: [PATCH 3/6] when opening a puzzle, move to the first across clue 
instead of the first white cell

In US-style crosswords, the first white cell is always in an across clue, but
this assumption breaks in British-style crosswords.
---
 xword |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/xword b/xword
index da69d8c..2bcd2b5 100755
--- a/xword
+++ b/xword
@@ -946,9 +946,7 @@ class PuzzleController:
         self.selection = []
 
         self.mode = ACROSS
-        (x, y) = (0, 0)
-        if puzzle.is_black(x, y):
-            ((x, y), _) = puzzle.next_cell(0, 0, ACROSS, 1, True)
+        x, y = self.puzzle.number_map[self.puzzle.first_number(ACROSS)]
         self.move_to(x, y)
 
     def connect(self, ev, handler):
-- 
1.6.2.4

>From d8b3cf71ceea05b765f4379867d45a295b6beb7a Mon Sep 17 00:00:00 2001
From: Dafydd Harries <d...@rhydd.org>
Date: Sun, 3 May 2009 17:24:53 +0300
Subject: [PATCH 4/6] don't switch mode when moving unless the current cell 
exists in the other mode

In British-style puzzles, some cells only exist in one of the two modes.
---
 xword |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/xword b/xword
index 2bcd2b5..996870d 100755
--- a/xword
+++ b/xword
@@ -1040,7 +1040,7 @@ class PuzzleController:
             ((x, y), _) = self.puzzle.next_cell(self.x, self.y,
                                                 self.mode, amt, skip_black)
             self.move_to(x, y)
-        else:
+        elif (self.x, self.y) in self.puzzle.mode_maps[1 - self.mode]:
             self.switch_mode()
 
     def back_space(self):
-- 
1.6.2.4

>From a886fea203e96e6fd0ec1c15170867e4943215c6 Mon Sep 17 00:00:00 2001
From: Dafydd Harries <d...@rhydd.org>
Date: Sun, 3 May 2009 17:24:25 +0300
Subject: [PATCH 5/6] ignore keypresses that can't be named

---
 xword |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/xword b/xword
index 996870d..a0775c6 100755
--- a/xword
+++ b/xword
@@ -1700,6 +1700,8 @@ class PuzzleWindow:
 
     def puzzle_key_event(self, item, event):
         name = gtk.gdk.keyval_name(event.keyval)
+        if name is None:
+            return False
         c = self.control
         if len(name) is 1 and name.isalpha():
             c.input_char(self.skip_filled, name)
-- 
1.6.2.4

>From 068c8f2d17e26a57a8ac35e3679a12aea6fb2e41 Mon Sep 17 00:00:00 2001
From: Dafydd Harries <d...@rhydd.org>
Date: Sun, 3 May 2009 19:46:18 +0300
Subject: [PATCH 6/6] add a rudimentary test suite

---
 test.py |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 128 insertions(+), 0 deletions(-)
 create mode 100644 test.py

diff --git a/test.py b/test.py
new file mode 100644
index 0000000..6d558c8
--- /dev/null
+++ b/test.py
@@ -0,0 +1,128 @@
+
+import unittest
+
+def read_file(path):
+    fh = file(path)
+    contents = fh.read()
+    fh.close()
+    return contents
+
+def load_module(name, path):
+    # hacketty hack
+
+    mod = __builtins__.__class__(name)
+    code = compile(read_file(path), path, 'exec')
+
+    g, l = {}, {}
+    exec code in g, l
+
+    for k, v in l.iteritems():
+        setattr(mod, k, v)
+
+    return mod
+
+xword = load_module('xword', 'xword')
+
+class TestPuzzle(xword.Puzzle):
+    def __init__(self):
+        # Override Puzzle.__init__ to avoid having to read a file.
+        pass
+
+def dump_map(width, height, m):
+    return ''.join([
+        ' '.join([
+            '%s' % m.get((x, y), '#')
+            for x in xrange(width)]) + '\n'
+        for y in xrange(height)])
+
+class SetupTest(unittest.TestCase):
+    def test_american(self):
+        puzzle = TestPuzzle()
+        puzzle.width = 5
+        puzzle.height = 5
+        puzzle.clues = [
+            'a1', 'd1', 'd2', 'd3', 'a4', 'd5', 'a6', 'd7', 'a8', 'a9']
+        puzzle.responses = dict([
+            ((x, y), ' .'[int(x + y == 4 and x != 2 and y != 2)])
+            for x in xrange(5)
+            for y in xrange(5)])
+
+        puzzle.setup()
+
+        self.assertEquals([], puzzle.clues)
+
+        self.assertEquals(
+            {1: True, 2: False, 3: False, 4: True, 5: False, 6: True, 7: False,
+             8: True, 9: True},
+            puzzle.is_across)
+        self.assertEquals(
+            {8: 'a8', 1: 'a1', 4: 'a4', 6: 'a6', 9: 'a9'},
+            puzzle.across_clues)
+        self.assertEquals(
+            '1 1 1 1 #\n'
+            '4 4 4 # #\n'
+            '6 6 6 6 6\n'
+            '# # 8 8 8\n'
+            '# 9 9 9 9\n',
+            dump_map(puzzle.width, puzzle.height, puzzle.across_map))
+
+        self.assertEquals(
+            {1: True, 2: True, 3: True, 4: False, 5: True, 6: False, 7: True,
+             8: False, 9: False},
+            puzzle.is_down)
+        self.assertEquals(
+            {1: 'd1', 2: 'd2', 3: 'd3', 5: 'd5', 7: 'd7'},
+            puzzle.down_clues)
+        self.assertEquals(
+            '1 2 3 # #\n'
+            '1 2 3 # 5\n'
+            '1 2 3 7 5\n'
+            '1 # 3 7 5\n'
+            '# # 3 7 5\n',
+            dump_map(puzzle.width, puzzle.height, puzzle.down_map))
+
+    def test_british(self):
+        puzzle = TestPuzzle()
+        puzzle.width = 5
+        puzzle.height = 5
+        puzzle.clues = ['a1', 'd1', 'd2', 'd3', 'a4', 'a5']
+        puzzle.responses = dict([
+            ((x, y), ' .'[int(x % 2 != 0 and y % 2 != 0)])
+            for x in xrange(5)
+            for y in xrange(5)])
+
+        puzzle.setup()
+
+        self.assertEquals([], puzzle.clues)
+
+        self.assertEquals(
+            {1: True, 2: False, 3: False, 4: True, 5: True},
+            puzzle.is_across)
+        self.assertEquals(
+            {1: 'a1', 4: 'a4', 5: 'a5'},
+            puzzle.across_clues)
+        self.assertEquals(
+            '1 1 1 1 1\n'
+            '# # # # #\n'
+            '4 4 4 4 4\n'
+            '# # # # #\n'
+            '5 5 5 5 5\n',
+            dump_map(puzzle.width, puzzle.height, puzzle.across_map))
+
+        self.assertEquals(
+            {1: True, 2: True, 3: True, 4: False, 5: False},
+            puzzle.is_down)
+        self.assertEquals(
+            {1: 'd1', 2: 'd2', 3: 'd3'},
+            puzzle.down_clues)
+        self.assertEquals(
+            '1 # 2 # 3\n'
+            '1 # 2 # 3\n'
+            '1 # 2 # 3\n'
+            '1 # 2 # 3\n'
+            '1 # 2 # 3\n',
+            dump_map(puzzle.width, puzzle.height, puzzle.down_map))
+
+if __name__ == '__main__':
+    unittest.main()
+
-- 
1.6.2.4


--- End Message ---

-- 
-John Sullivan
-http://wjsullivan.net
-GPG Key: AE8600B6

Reply via email to