Re: [O] Patch to implement sorting Org tables by IP address

[Nicolas Goaziou]

Jon Snader <address@hidden> writes:

>> On Dec 14, 2014, at 12:18 PM, Nicolas Goaziou <address@hidden> wrote:
>
>>> As I said above, you’ve convinced me that ?f ?F is the right solution.
>> 
>> Fair enough. Let's settle on that, then.
>
> Here is the new patch. It extends org-table-sort-lines to allow a user
> to specify custom extraction and comparison functions using the ?f ?F
> sorting type as in org-sort-list.
>
> The user can call org-table-sort-lines programmatically specifying
> a SORTING-TYPE of ?f or ?F and provide custom extraction and
> comparison functions. If the user calls org-table-sort lines
> interactively and specifies ?f or ?F, org-do-sort will prompt for
> a custom extraction function. The comparison will be either string or
> numeric depending on the type of the first extracted key.

Thank you. Some comments follow.

> -any of (?a ?A ?n ?N ?t ?T) where the capital letter indicate that sorting
> -should be done in reverse order."
> +any of (?a ?A ?n ?N ?t ?T ?f ?F) where the capital letter indicate
> that sorting

"indicates" or "capital letters"

> +     extractfun comparefun tempfun extract-string-p)

EXTRACT-STRING-P, and possibly TEMPFUN, are bound too early. See below.

>      ;; Define the appropriate functions
>      (cond
>       ((= dcst ?n)
> @@ -9075,7 +9081,7 @@ If WITH-CASE is non-nil, the sorting will be 
> case-sensitive."
>        (setq extractfun (if with-case (lambda(x) (org-sort-remove-invisible 
> x))
>                        (lambda(x) (downcase (org-sort-remove-invisible x))))
>           comparefun (if (= dcst sorting-type)
> -                        'string<
> +                        #'string<

OK, but then there are other occurrences in the function to modify.
I suggest to remove this.

> +     ((= dcst ?f)
> +      (setq tempfun (or getkey-func
> +                     (intern (org-icompleting-read
> +                              "Sort using function: "
> +                              obarray 'fboundp t nil nil))))
> +      (setq extract-string-p (stringp (funcall tempfun (caar table))))
> +      (setq extractfun (if (and extract-string-p (not with-case))
> +                        (lambda (x) (downcase (funcall tempfun x)))
> +                      tempfun))
> +      (setq comparefun (or compare-func
> +                        (if extract-string-p 
> +                            (if (= sorting-type ?f)
> +                                #'string<
> +                              (lambda (a b) (and (not (string< a b))
> +                                                 (not (string= a b)))))
> +                          (if (= sorting-type ?f)
> +                              #'<
> +                            #'>)))))

I suggest something like

  (let* ((tempfun (or getkey-func
                      (intern ...)))
         (extract-string-p (stringp (funcall tempfun (caar table)))))
    (setq extractfun (if (and extract-string-p (not with-case))
                         `(lambda (x) (downcase (funcall ',tempfun x)))
                       tempfun))
    (setq comparefun
         (cond (compare-func)
               (extract-string-p (if (= sorting-type ?f) #'string< 
#'org-string>))
               ((= sorting-type ?f) #'<)
               (t #'>))))


Regards,