bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Ivan Shmakov]

Package:  emacs
Severity: minor

	The eww.el code currently relies on plist-put modifying the
	property list given.  This mainly concerns eww-data (as of the
	recent changes), but occasionally also other variables.

	This feature is, however, undocumented.  For instance, both the
	docstring (src/fns.c) and the reference (doc/lispref/lists.texi)
	suggest an explicit (setq var (plist-put var prop val)) form
	instead, and lispref/lists.texi in particular clearly omits the
	specific conditions under which the list passed will be modified
	in-place:

    […] It may modify plist destructively, or it may construct a new
    list structure without altering the old.  […]

	Briefly scanning over eww.el, I see no actual bugs that may
	arise from that given the current plist-put implementation
	(which apparently returns a new list if and only if given nil as
	the argument, and modifies the list given otherwise.)  Still, I
	don’t feel it all that nice to rely on such an undocumented
	behavior.

	If this behavior is, however, well-known by now, I guess it
	really deserves to be documented.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Lars Magne Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> Briefly scanning over eww.el, I see no actual bugs that may
> arise from that given the current plist-put implementation
> (which apparently returns a new list if and only if given nil as
> the argument, and modifies the list given otherwise.) 

It should be documented.

On the other hand, eww should really be using an object for this stuff
and not a plist.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Stefan Monnier]

> On the other hand, eww should really be using an object for this stuff
> and not a plist.

Make it hash table,


        Stefan

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Ivan Shmakov]

>>>>> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> writes:

 >> Briefly scanning over eww.el, I see no actual bugs that may arise
 >> from that given the current plist-put implementation (which
 >> apparently returns a new list if and only if given nil as the
 >> argument, and modifies the list given otherwise.)

 > It should be documented.

 > On the other hand, eww should really be using an object for this
 > stuff and not a plist.

	What for, specifically?

	For instance, the current (property list) version of the
	eww-desktop-data-1 filter is as follows:

(defun eww-desktop-data-1 (alist)
  (let ((acc  nil)
        (tail alist))
    (while tail
      (let ((k (car  tail))
            (v (cadr tail)))
        (when (memq k eww-desktop-data-save)
          (setq acc (cons k (cons v acc)))))
      (setq tail  (cddr tail)))
    acc))

	The association list version could be as simple as:

(defun eww-desktop-data-1 (alist)
  (remove-if-not (lambda (k) (memq k eww-desktop-data-save))
                 alist
                 :key 'car))

	And for eww-data and eww-history to be object-based, I can’t
	readily suggest any simple implementation.

	(For a hash table, it could presumably be something based on
	maphash.)

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> On the other hand, eww should really be using an object for this stuff
>> and not a plist.
>
> Make it hash table,

That would also work.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Lars Magne Ingebrigtsen]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>>> On the other hand, eww should really be using an object for this stuff
>>> and not a plist.
>>
>> Make it hash table,
>
> That would also work.

Geez.  We have real hash tables now in Emacs?  Looking at the code, it
was added in 1999, but I think it's managed to escape my attention until
now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Stefan Monnier]

> Geez.  We have real hash tables now in Emacs?

It's a recent addition, new in Emacs-21.1.

> Looking at the code, it was added in 1999, but I think it's managed to
> escape my attention until now.

Welcome to the 20th century!


        Stefan

bug#19104: eww.el vs. plist-put documentation (docstring, doc/lispref/lists.texi)

[Lars Ingebrigtsen]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Ivan Shmakov <ivan@siamics.net> writes:
>
>> Briefly scanning over eww.el, I see no actual bugs that may
>> arise from that given the current plist-put implementation
>> (which apparently returns a new list if and only if given nil as
>> the argument, and modifies the list given otherwise.) 
>
> It should be documented.
>
> On the other hand, eww should really be using an object for this stuff
> and not a plist.

But I don't think there's a bug here.  If somebody wants to rewrite eww
to use an object or a hash table, that's fine by me.  Alists, not so much.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no