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<address@hidden>
* 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.
5x5.el.diff-suggestion