Hi Nicolas,

thanks for the review. I hope the new version is an improvement.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Also, note that you can avoid requiring MOVE-FN, ELEMENT and CMP-FN if
> you decide that
>
>   (< n 0)  => (#'org-table-next-field :contents-end #'<=)
>   (> n 0)  => (#'org-table-previous-fierd :contents-begin #'>=)
>
> Up to you.

I wanted to use the n=0 case to supress the conditional movement to the
next field. It's probably not worth it and I removed it. Now everything
simplifies to one function.

> IOW, you need to let-bind (org-element-context) and find the first
> `table', `table-row' or `table-cell' object/element among it and its
> parents. Then
>
>   - if no such ancestor is found: return an error (not at a table)
>
>   - if `table' is found but point is not within
>     [:contents-begin :contents-end[ interval, return an error (not
>     inside the table)
>
>   - if `table' or `table-row' is found, you need to apply
>     org-table/previous/next/-field once (and diminish N by one) to make
>     sure point will be left on a regular cell, if possible.

But as long as I have a table cell ancestor, I should be fine. Am I
missing something? I added a function `org-element-get' to help with
that. What do you think? (If `cl-labels' is a no-go, we could split out
a helper function.)

> Thank you for taking care of this. There are bonus points if you can
> write tests along with this change.

I added a couple of tests. Not really succinct, though.

Also, I modified `org-element-table-cell-parser', because otherwise
:contents-begin and :contents-end point to the end of the cell. Not sure
if this breaks anything.

-- 
Florian Beck