[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#47784: 27.2: ses-header-row [PATCH INCLUDED]
From: |
Lars Ingebrigtsen |
Subject: |
bug#47784: 27.2: ses-header-row [PATCH INCLUDED] |
Date: |
Wed, 05 May 2021 15:08:52 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Boruch Baum <boruch_baum@gmx.com> writes:
> Two patch files are attached.
>
> + ses-header-row.patch
>
> + Function `ses-set-header-row' was determining the current row based
> upon variable `ses--curcell', but that variable is NIL until one
> begins an operation on a cell (eg. keybindings '=', '"'), so
> navigating to a row was insufficient to select that row, and further
> generated an ERROR because the code was not expecting a NIL value for
> variable `ses--curcell'
>
> + Using function `user-error' instead of `error' doesn't spawn a
> debug session and backtrace buffer when variable
> `toggle-debug-on-error' is set.
>
> + ses-user-error.patch
>
> + Quasi-randomly replaces some calls to function `error' with
> `user-error'.
Looks good to me, so I've applied both patches to Emacs 28. Eli had
some questions about the usage of user-error instead of error in the
second patch, but as far as I can tell, using `user-error' is indeed the
right thing here, because the errors in question seem more user
initiated.
I've got two tiny comments about the patch. First of all, the
whitespace seemed odd:
lisp/ses.el:3462: space before tab in indent.
+ (currow (1+ (car (ses-sym-rowcol curcell)))))
So I fixed that up, but I've seen that in other patches -- could you
have a look and see what's causing this mixture of tab and spaces?
The other comment is that it would be nice to have ChangeLog-style
commit messages -- use `C-x 4 a' to format one before sending patches.
Less work when applying the patch. :-)
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
- bug#47784: 27.2: ses-header-row [PATCH INCLUDED],
Lars Ingebrigtsen <=
bug#47784: 27.2: ses-header-row [PATCH INCLUDED], Lars Ingebrigtsen, 2021/05/06