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.