emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [DRAFT PATCH v2] Decouple LANG= and testing (was: Test failure due t


From: Ruijie Yu
Subject: Re: [DRAFT PATCH v2] Decouple LANG= and testing (was: Test failure due to LANG)
Date: Tue, 25 Apr 2023 23:14:36 +0800
User-agent: mu4e 1.9.22; emacs 30.0.50

Ihor Radchenko <yantar92@posteo.net> writes:

>> +(defun org-columns--substring-below-width (string start width)
>> +  "Similar to `substring', but use `string-width' to check width.
>
> This is not really similar to `substring' as `substring' has totally
> different third argument.

Addressed in v6 -- see my update in the other subthread.

>> +The returned value is a substring of STRING, starting at START,
>> +and is the largest possible substring whose width does not exceed
>> +WIDTH."
>> +  (let ((end (min (+ start width) (length string))) res)
>> +    (while (and end (>= end start))
>> +      (let* ((curr (string-width string start end))
>> +             (excess (- curr width)))
>> +        (if (cl-plusp excess)
>
> Why not simply (> excess 0)? `cl-plusp' is a bit confusing - we
> generally avoid cl-lib functions unless necessary. (I've never seen
> `cl-plusp' used frequently)

Well, one of the courses I took last year used Common Lisp, where plusp
and minusp were largely preferred over (> x 0), hence the habit.  I have
switched over accordingly in v6.

>>  (defun org-columns-add-ellipses (string width)
>>    "Truncate STRING with WIDTH characters, with ellipses."
>>    (cond
>> -   ((<= (length string) width) string)
>> -   ((<= width (length org-columns-ellipses))
>> -    (substring org-columns-ellipses 0 width))
>> -   (t (concat (substring string 0 (- width (length org-columns-ellipses)))
>> -          org-columns-ellipses))))
>> +   ((<= (string-width string) width) string)
>> +   ((<= width (string-width org-columns-ellipses))
>> +    (org-columns--substring-below-width org-columns-ellipses 0 width))
>> +   (t (concat
>> +       (org-columns--substring-below-width
>> +        string 0 (- width (length org-columns-ellipses)))
>> +       org-columns-ellipses))))
>
> It will be best to write dedicated tests here that will clearly indicate
> issues when some non-standard LANG environment is used. The current
> failure is rather difficult to debug.

Done -- somewhat.  At the time I wrote the tests I misunderstood what
you said, so now we have a test on org-columns--truncate-below-width
(renamed from org-columns--substring-below-width).  I can add a test on
org-columns-add-ellipses as well, if you think it is necessary.  But
probably tomorrow.

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]



reply via email to

[Prev in Thread] Current Thread [Next in Thread]