Could you please send me the diff compared to my contribution, rather than compared to the official latest 5x5, that would help me understand your suggestion. VBR, Vincent. > Date: Sat, 21 May 2011 11:36:52 -0300 > From: viniciusjl@ig.com.br > To: vincent.b.1@hotmail.fr > Subject: Re: 5x5 Arithmetic solver > CC: monnier@iro.umontreal.ca; emacs-devel@gnu.org > > Hi Vincent, > > Could I suggest the attached diff? > > Vinicius > > > On 05/21/2011 04:15 AM, Vincent Belaïche wrote: > > Salut Stéfan, > > > > Toujours aussi à cheval sur les conventions de codage ! > > > >>> Please find herein attached a contribution to the 5x5 game. This is an > >>> arithmetic solver based on a matrix inversion in a (Z/2Z)^25 vector > >>> space. > >> Thanks. A few comments: > >> - avoid using a tarball and just attach the diff as-is, > >> makes it a lot easier to review. > > noted > > > >> - why 5x5-local-variables? > > That is used in 5x5-mode to imply that all listed variable are made > > local to that very 5x5 buffer. if after `M-x 5x5' you do a `M-x > > rename-uniquely' and then `M-x 5x5' again then you have have two > > independent games. > > > >> - explain the changes in the 5x5 function. > > I had to change slightly the order of operations because setting the > > mode has to be done before any buffer local 5x5 variable is touched, as > > precisely those variables are made local by setting the mode > > > >> - many of your lines have trailing whitespace. I generally don't care > >> much, about it, but some people do, and it's usually preferable to > >> avoid them. M-x picture-mode C-c C-c gets rid of them for you (as > >> a side-effect). > > Done > > > >> - try to keep the first line of docstrings as a self-sufficient sentence > >> (because M-x apropos only shows the first line). > > > > > >> - stay within 80 columns. > > Do you mean that we are still in the 80ies ;-P ? > > > > Ok, done. > > > >> - your code is not properly indented (e.g. the `grid' argument in > >> 5x5-grid-to-vec). > > Done > > > >> - Please capitalize your comments and terminate them with a "." or some > >> other appropriate punctuation. > > Some comments by a variable name, so they are not capitalized. > > > > Some comments are equations like this: > > > > ;; B:= targetv > > ;; A:= transferm > > ;; P:= base-change > > ;; P^-1 := inv-base-change > > ;; X := solution > > > > ;; B = A * X > > ;; P^-1 * B = P^-1 * A * P * P^-1 * X > > ;; CX = P^-1 * X > > ;; CA = P^-1 * A * P > > ;; CB = P^-1 * B > > ;; CB = CA * CX > > ;; CX = CA^-1 * CB > > ;; X = P * CX > > > > I don't think that this is common practice to use a punctuation at the > > end of an equation. > > > > Some other comment is commented out code like this: > > > > ;;(ctransferm-1-2 (calcFunc-mcol ctransferm-1-: col-2)) > > > > This code is not there because I don't need that variable, but I found > > useful to show how it would be defined. > > > > For all the remaining comments I have done it > > > >> - 5x5-solve-suggest should have a docstring. > > Done > > > >> - try C-u checkdoc-current-buffer. > > I get > > > > checkdoc-continue: Too many occurrences of \[function]. Use \{keymap} > > instead > > > > Because 5x5 is used many times as if it was > > > > > >> - we need a ChangeLog entry. > > Here it is: > > > > ----------------------------------------------------------------------- > > 2011-05-21 Vincent Belaïche > > > > * play/5x5.el: Add an arithmetic solver to suggest positions to > > click on. > > ----------------------------------------------------------------------- > > > >> - I don't understand the "solve step" message (e.g. it said 23 every > >> time, even though I followed its suggestions and finished in 12 moves). > >> > >> > > Ask it to Jay, this message is output by Calc, not by 5x5. 23 is due > > to this that you have to invert a 23x23 matrix. Altough the 5x5 transfer > > matrix is 25x25, its rank is only 23, so I extract some submatrix to > > compute the solution. > > > >> Stefan > > Vincent. > > >