Subject: Re: bug#32372: [PATCH] Add "uuid" to thing-at-point.el

From: Ivan Shmakov <[email-masked]>
Date: Tue, 07 Aug 2018 13:17:21 +0000

>>>>> Raimon Grau <[email-masked]> writes:

	A few minor points.

[…]
 
 > +---
 > +** thingatpt.el supports a new "thing" called 'uuid'.
 > +
 > +A symbol 'uuid' can be passed to thing-at-point and it returns the
 > +uuid at point.

	I think the latter UUID should be spelled in all-caps.

[…]

 > +;; UUID
 > +
 > +(defvar thing-at-point-uuid-regexp

	There seem to be no precedent on the use of defconst in
	thingatpt.el, but given that the UUID format is ought to be
	stable, I guess this would be exactly the place for one.  Or?

 > +  (rx bow
 > +      (repeat 8 hex-digit) "-"
 > +      (repeat 4 hex-digit) "-"
 > +      (repeat 4 hex-digit) "-"
 > +      (repeat 4 hex-digit) "-"
 > +      (repeat 12 hex-digit)
 > +      eow)
 > +  "A regular expression matching a UUID.
 > +
 > +  More info on uuid's format in
 > +  https://tools.ietf.org/html/rfc4122." )

	AIUI, the docstrings are not indented like that; also, there
	should be no blank before the closing parenthesis.

	Given that there seem to be no URL references in thingatpt.el
	docstrings, either, I’d rather rewrite this one as:

+  "A regular expression matching a UUID.
+
+See RFC 4122 for the description of the format.")

 > +
 > +(put 'uuid 'bounds-of-thing-at-point
 > +     (lambda ()
 > +       (let ((thing (thing-at-point-looking-at
 > +                     thing-at-point-uuid-regexp 36)))
 > +         (if thing
 > +             (let ((beginning (match-beginning 0))
 > +                   (end (match-end 0)))
 > +               (cons beginning end))))))

	Why not simplify to (cons (match-beginning 0) (match-end 0))?

 > +
 >  ;;  Aliases

 >  (defun word-at-point ()

[…]

-- 
FSF associate member #7257  http://am-1.org/~ivan/