Hi Michael,

Thanks for the review! Updated patches attached.  I believe I've fixed
everything you mentioned - let me know if I missed something.

On Sun, Oct 19, 2014 at 3:57 PM, Michael Brand <michael.ch.br...@gmail.com>
wrote:

> Hi Nathaniel
>
> On Sat, Oct 18, 2014 at 7:11 AM, Nathaniel Flath <flat0...@gmail.com>
> wrote:
> > Patches are attached.
>
> I am not an expert for all the following comments, please correct me
> or contradict where necessary.
>
> The patches do not apply on current master, so I did not try them out
> yet.
>
> You might want to add a def-edebug-spec like there is one for many
> other defmacro in Org.
>

Limit lines to max. 80 chars.
>
> It will make it easier for the maintainer Bastien to apply the patches
> when you format them with git including a changelog etc. as described
> here
> http://orgmode.org/worg/org-contribute.html
>

> > +          (if (not all) (message "Re-applying formula to field: %s"
> (car eq))
> > +            (org-table-execute-once-per-second log-last-time (message
> "Re-applying formula to field: %s" (car eq))))
>
> Good idea to still log always when only one table row is recalculated.
>
> The doubling of the message makes it more complicated to maintain its
> string. I suggest to change the macro to allow
>
>     (org-table-execute-once-per-second
>      (when all log-last-time)  ; Log just always when `all' is nil.
>      (message "Re-applying formula to field: %s" (car eq)))



> Why not test `all' also for the other message with "to field"?
>
>

> > +       ,@body
> > +       )))
>
>
> > +            (and all (org-table-execute-once-per-second log-last-time
> (message "Re-applying formulas to %d lines...done" cnt))))
>
> Shouldn't this use `log-first-time'?
>
>


> Michael
>

Attachment: 0001-org-table.el-Add-early-return-check-to-org-table-rec.patch
Description: Binary data

Attachment: 0002-org-table.el-Print-far-fewer-messages-when-recalcula.patch
Description: Binary data

Reply via email to