Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

> +(defun frameset-p (object)
> +  "If OBJECT is a frameset, return its version number.

I know we have some offenders, but please try to stick to "foo-p returns
a boolean".

>  Else return nil."
> -  (and (eq (car-safe frameset) 'frameset) ; is a list
> -       (integerp (nth 1 frameset)) ; version is an int
> -       (nth 3 frameset)            ; states is non-null
> -       (nth 1 frameset)))          ; return version
> +  (and (vectorp object)                   ; a vector
> +       (eq (aref object 0) 'frameset)     ; tagged as `frameset'
> +       (integerp (aref object 1))         ; version is an int
> +       (consp (aref object 2))            ; timestamp is a non-null list
> +       (stringp (or (aref object 4) ""))  ; name is a string or null
> +       (stringp (or (aref object 5) ""))  ; description is a string or null
> +       (listp (aref object 6))            ; properties is a list
> +       (consp (aref object 7))            ; and states is non-null
> +       (aref object 1)))                  ; return version

Also, this doesn't just test whether it's a frameset or not (which is
what one expects from "frameset-p") but it does some sanity checks
as well, something I'd call maybe "frameset-valid-p".


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 3:56 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I know we have some offenders, but please try to stick to "foo-p returns
> a boolean".

Why?  It's a CL-style predicate, which returns a "generalized boolean"
(either nil, or anything else). What is the harm in returning the
version?

> Also, this doesn't just test whether it's a frameset or not (which is
> what one expects from "frameset-p") but it does some sanity checks
> as well, something I'd call maybe "frameset-valid-p".

It needs to be called `frameset-p' for cl-typep to work.

If you're suggesting that I split the current frameset-p into a
strict-boolean-returning frameset-p and a more thorough
frameset-valid-p, I can do that, but, what is the gain?

   J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

>> I know we have some offenders, but please try to stick to "foo-p returns
>> a boolean".
> Why?  It's a CL-style predicate, which returns a "generalized boolean"
> (either nil, or anything else).  What is the harm in returning the
> version?

I don't really care that it doesn't always return t as non-nil value.
But its non-nil value should be treated by its docstring (and by
callers) as a boolean equivalent to t.  Otherwise it's not just
a "predicate" but an accessor.

>> Also, this doesn't just test whether it's a frameset or not (which is
>> what one expects from "frameset-p") but it does some sanity checks
>> as well, something I'd call maybe "frameset-valid-p".
> It needs to be called `frameset-p' for cl-typep to work.

You can use the built-in frameset-p (and then remove the :named
and :type as well, so that the tag becomes internal/hidden, which
I also find cleaner)).

> If you're suggesting that I split the current frameset-p into a
> strict-boolean-returning frameset-p and a more thorough
> frameset-valid-p, I can do that, but, what is the gain?

That's it's more idiomatic?


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 4:46 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I don't really care that it doesn't always return t as non-nil value.

OK.

> But its non-nil value should be treated by its docstring (and by
> callers) as a boolean equivalent to t.  Otherwise it's not just
> a "predicate" but an accessor.

Not in CL parlance: a predicate is "a function that returns a
generalized boolean as its first value." No other restrictions. It's
the callers' responsibility to treat its result as a boolean.

Yes, I'm aware this is elisp and not CL. Unfortunately.

> You can use the built-in frameset-p (and then remove the :named
> and :type as well, so that the tag becomes internal/hidden, which
> I also find cleaner)).

Both are hidden, as it is something that you don't ever see (except,
in this case, in the frameset-p predicate). But, if you ever happen to
see the tag, cl-struct-frameset is horrible. "cl-defstruct-" is jus
the implementation leaking.

  C:\> sbcl
  This is SBCL 1.1.4.0.mswin.1288-90ab477, an implementation of ANSI
Common Lisp.
  More information about SBCL is available at <http://www.sbcl.org/>.
  [...]
  * (defstruct frameset a b c)

  frameset
  * (make-frameset)

  #S(frameset :a nil :b nil :c nil)

which is much nicer (even after #S).

But the deeper question is, I really prefer to have a more checking
frameset-p. I want to discourage people of going the make-frameset
route and using frameset-save instead.

   J

> That's it's more idiomatic?

A predicate is something that checks inclusion in a type.
[cl-struct-frameset "Hello" "Goodbye" 'nothing 'to 'see 'here [0 0 0]]
is not a frameset, even if the built-in frameset-p thinks so.

    J

RE: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Drew Adams]

> Yes, I'm aware this is elisp and not CL.  Unfortunately.

No need to feel bad.  For Emacs Lisp too any non-nil object
represents true.

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

> Not in CL parlance: a predicate is "a function that returns a
> generalized boolean as its first value." No other restrictions.  It's
> the callers' responsibility to treat its result as a boolean.

So we agree: the return value should be treated as a boolean, even if it
may not always be restricted to t or nil.  So it shouldn't document its
return value as being something else.

> Both are hidden, as it is something that you don't ever see (except,
> in this case, in the frameset-p predicate). But, if you ever happen to
> see the tag, cl-struct-frameset is horrible. "cl-defstruct-" is jus
> the implementation leaking.

Actually, if we ever want to save those vectors to a file, then I agree
that `frameset' is indeed better than `cl-defstruct-frameset'.  I had
forgotten about this point.

> But the deeper question is, I really prefer to have a more checking
> frameset-p. I want to discourage people of going the make-frameset
> route and using frameset-save instead.

Then why don't you remove make-frameset and replace it with a good
constructor which can only build elements that agree with your tighter
constraints?  You wouldn't need to check those constraints in frameset-p
any more since they'd be true by construction.

>> That's it's more idiomatic?
> A predicate is something that checks inclusion in a type.

Indeed.  Not something that returns a version number.

> [cl-struct-frameset "Hello" "Goodbye" 'nothing 'to 'see 'here [0 0 0]]
> is not a frameset, even if the built-in frameset-p thinks so.

It's a frameset, it's just not a valid one.  If/when cl-defstruct is
extended to support type annotations on its slots, then I might agree with
you, but note that if this ever happens, the type checks will not be in
frameset-p but in the constructors and the setters.


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 6:09 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> So we agree: the return value should be treated as a boolean, even if it
> may not always be restricted to t or nil.  So it shouldn't document its
> return value as being something else.

Fixed; now references to `frameset-p', and its docstring, only mention
nil and non-nil values.

> Actually, if we ever want to save those vectors to a file, then I agree
> that `frameset' is indeed better than `cl-defstruct-frameset'.

We save those vectors to a file, in desktop.el. That's the whole point
of creating framesets.

> Then why don't you remove make-frameset and replace it with a good
> constructor which can only build elements that agree with your tighter
> constraints?  You wouldn't need to check those constraints in frameset-p
> any more since they'd be true by construction.

I sort of did. make-frameset is specifically not documented, and the
docstring for the type has this bit:

 - `frameset-save', the type's constructor, captures all or a subset of the
   live frames, and returns a serializable snapshot of them (a frameset).

The reason I didn't completely remove make-frameset is because I use
internally. It's a tiny bit cleaner that constructing the vector by
hand.

> Indeed.  Not something that returns a version number.

A generalized boolean :-)

> It's a frameset, it's just not a valid one.

I am not sure I agree that a bottle of water is a bottle of Vega
Sicilia, just an invalid one...

> If/when cl-defstruct is
> extended to support type annotations on its slots, then I might agree with
> you, but note that if this ever happens, the type checks will not be in
> frameset-p but in the constructors and the setters.

OK, it's your show, your rules. :-)

I've allowed again the built-in frameset-p, and renamed (and expanded)
the old one as frameset-valid-p. frameset-restore still uses the more
thorough one.

Also, I've added docstrings for frameset-p and all slot accessors, via
(put 'frameset-SLOT 'function-documentation "Docstring."). Oh the joy.
Please tell me that I just missed a way to define the docstrings in
the cl-defstruct declaration...

    J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

I just realized that a predicate function with this profile

  (defun frameset-p (object &optional valid-p)
    "Return non-nil if OBJECT is a frameset, nil otherwise.

  With VALID-P, return non-nil if OBJECT is a valid frameset
  \(i.e. check more thoroughly)."
    ...)

would also be kosher, at least in the CL tradition.

No, I'm not going to go that route, just an idle thought.
    J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

>  - `frameset-save', the type's constructor, captures all or a subset of the
>    live frames, and returns a serializable snapshot of them (a frameset).
> The reason I didn't completely remove make-frameset is because I use
> internally.  It's a tiny bit cleaner that constructing the vector by
> hand.

You could make it more clearly internal by renaming it to something like
frameset--super-dangerous-constructor-do-not-use.

You could also replace it with a constructor that does the right thing.
Something like

   (:constructor nil)
   (:constructor
    frameset-make
    (frame-list &key app name description
                     filters predicate properties
                &aux (states
                      (let* ((list (or (copy-sequence frame-list)
                                       (frame-list)))
	                     (frames (cl-delete-if-not
                                      #'frame-live-p
				      (if predicate
				          (cl-delete-if-not predicate list)
				        list))))
                        (frameset--record-minibuffer-relationships frames)
                        (mapcar
			 (lambda (frame)
			   (cons
			    (frameset-filter-params (frame-parameters frame)
						    (or filters
          					        frameset-filter-alist)
						    t)
			    (window-state-get (frame-root-window frame) t)))
			 frames)))))

But I find this way really hideous.

> Also, I've added docstrings for frameset-p and all slot accessors, via
> (put 'frameset-SLOT 'function-documentation "Docstring."). Oh the joy.
> Please tell me that I just missed a way to define the docstrings in
> the cl-defstruct declaration...

No, I don't think there's such a thing as slot-doc-strings ;-)
Neither in cl-defstruct, nor in Common-Lisp itself.


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 3:18 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> You could make it more clearly internal by renaming it to something like
> frameset--super-dangerous-constructor-do-not-use.

Well, I could, but I expect users of the package will take the hint
that everything else is documented and make-frameset is not.

> You could also replace it with a constructor that does the right thing.
> Something like
  [...]
> But I find this way really hideous.

"Hideous" is too nice a word.

> No, I don't think there's such a thing as slot-doc-strings ;-)
> Neither in cl-defstruct, nor in Common-Lisp itself.

Well, (put 'SLOT 'function-doctstring "Doc") is then.

BTW, how do I mark the built-in frameset-p as autoloaded?

    J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

>> You could make it more clearly internal by renaming it to something like
>> frameset--super-dangerous-constructor-do-not-use.
> Well, I could, but I expect users of the package will take the hint
> that everything else is documented and make-frameset is not.

BTW, I don't see anything in frameset-save which guarantees that it
returns a valid frameset.

>> No, I don't think there's such a thing as slot-doc-strings ;-)
>> Neither in cl-defstruct, nor in Common-Lisp itself.
> Well, (put 'SLOT 'function-doctstring "Doc") is then.
> BTW, how do I mark the built-in frameset-p as autoloaded?

I don't think there's a way to do that.  Why do you need it?


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 4:40 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> BTW, I don't see anything in frameset-save which guarantees that it
> returns a valid frameset.

If you mean that app, name, description and properties are not
checked, er... good catch ;-)  (states, version and timestamp cannot
be wrong unless something fails catastrophically).

> I don't think there's a way to do that.  Why do you need it?

frameset-p, frameset-save and frameset-restore are the functions more
likely to be used from outside packages; it's good to have them
autoloaded to avoid require'ing unnecessarily.

Currently:

 emacs -Q
 M-: (set-register ?a '(file . "~/.emacs")) <RET>
 C-x r j a
 => Symbol's function definition is void: frameset-p

so I'll have to add a (require 'frameset), which is a pity.

     J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

>> I don't think there's a way to do that.  Why do you need it?

> frameset-p, frameset-save and frameset-restore are the functions more
> likely to be used from outside packages; it's good to have them
> autoloaded to avoid require'ing unnecessarily.

> Currently:

>  emacs -Q
>  M-: (set-register ?a '(file . "~/.emacs")) <RET>
>  C-x r j a
>  => Symbol's function definition is void: frameset-p

> so I'll have to add a (require 'frameset), which is a pity.

Hmm... why don't you use registerv-make (and get rid of the
frameset-specific code in register.el)?  That's what it was added for.


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 6:32 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Hmm... why don't you use registerv-make

I didn't know about it.

> (and get rid of the frameset-specific code in register.el)?

I like that idea very much, but I see a problem. We're replacing
frame-configuration-to-register by frameset-to-register, which
includes the delete-or-iconify behavior controled by `C-u C-x r j'
when the register contains a frame-configuration.

I see no way to pass that to registerv's jump-func. Or is there one?

   J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

> I like that idea very much, but I see a problem. We're replacing
> frame-configuration-to-register by frameset-to-register, which
> includes the delete-or-iconify behavior controled by `C-u C-x r j'
> when the register contains a frame-configuration.
> I see no way to pass that to registerv's jump-func. Or is there one?

If there's no way, currently, then maybe it's a good opportunity to
add one.


        Stefan

RE: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Drew Adams]

> frameset-p, frameset-save and frameset-restore are the functions more
> likely to be used from outside packages

frameset-p or frameset-valid-p?

 - Lasciate ogne speranza, voi ch'intrate

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 6:50 PM, Drew Adams <drew.adams@oracle.com> wrote:

> frameset-p or frameset-valid-p?

Also frameset-valid-p, but that function is already autoloaded.

    J

RE: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Drew Adams]

> > frameset-p or frameset-valid-p?
> 
> Also frameset-valid-p, but that function is already autoloaded.

"Also"?

Why would we ever want someone to use `frameset-p'?  What's the
point of testing only for a possibly invalid structure that
happens to have the right tag?

Not a rhetorical question.  Do we really want people relying
on `frameset-p' (and `cl-typep') to check whether something is
of type frameset?

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

>> > frameset-p or frameset-valid-p?
>> Also frameset-valid-p, but that function is already autoloaded.
> Not a rhetorical question.  Do we really want people relying
> on `frameset-p' (and `cl-typep') to check whether something is
> of type frameset?

I do.


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 7:18 PM, Drew Adams <drew.adams@oracle.com> wrote:

> "Also"?
>
> Why would we ever want someone to use `frameset-p'?  What's the
> point of testing only for a possibly invalid structure that
> happens to have the right tag?

In some context might make sense. For frameset-to-register, for
example, the frameset-p check is not to check the frameset's validity,
just that it is of the right type, because the frameset was generated
under controlled circumstances and cannot be erroneous. If the user
has a sudden impulse to do

M-: (set-register ?a (make-frameset nil :states 'hawhaw)) <RET>
C-x r j a

s/he will get what s/he asked for. No checking will protect against
any erroneous data, after all; all the frameset-valid-p in the world
won't protect the user that wants to do

(let ((circ '(nil))
      (fs (frameset-save nil)))
  (setcdr circ circ)
  (setf (frameset-states fs) circ)
  fs)

> Not a rhetorical question.  Do we really want people relying
> on `frameset-p' (and `cl-typep') to check whether something is
> of type frameset?

You've been following this conversation and know that my preference
would be for a strong-checking frameset-p. Given that it is not going
to be so, I can abuse the implementation of cl-typep and teach the
users to do (cl-typep FRAMESET 'frameset-valid), which happens to work
;-)

   J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 7:47 PM, Juanma Barranquero <lekktu@gmail.com> wrote:

> No checking will protect against
> any erroneous data, after all

s/any/every possible/

of course.

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 4:40 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I don't think there's a way to do that.

Hmm, there's no reason why this wouldn't work:

;;;###autoload
(autoload 'frameset-p "frameset" "Return non-nil if OBJECT is a
frameset, nil otherwise." nil)

and, though ugly, is still prettier than having to require 'frameset
from register.el (which has to be done at the point of call of
frameset-p, because register.el is preloaded).

WDYT?

   J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

>> I don't think there's a way to do that.

> Hmm, there's no reason why this wouldn't work:

> ;;;###autoload
> (autoload 'frameset-p "frameset" "Return non-nil if OBJECT is a
> frameset, nil otherwise." nil)

Of course.  You could also do it as:

;;;###autoload (autoload 'frameset-p "frameset"
;;;###autoload   "Return non-nil if OBJECT is a frameset, nil otherwise." nil)

which has the advantage tht this code is not run when loading frameset.
Or you could do it by replacing the declare-function call (in
register.el) by a call to `autoload'.

> and, though ugly, is still prettier than having to require 'frameset
> from register.el (which has to be done at the point of call of
> frameset-p, because register.el is preloaded).

> WDYT?

Agreed.  I only meant to say that there's no way to do it "cleanly"
(e.g. such that the arguments to `autoload' are constructed
automatically from contextual information).


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 7:30 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> ;;;###autoload (autoload 'frameset-p "frameset"
> ;;;###autoload   "Return non-nil if OBJECT is a frameset, nil otherwise." nil)

> which has the advantage tht this code is not run when loading frameset.

OK, I'll do that.

> Or you could do it by replacing the declare-function call (in
> register.el) by a call to `autoload'.

Well, register.el is preloaded so that would make frameset-p
autoloadable in emacs -Q. But it seems cleaner to me to have this
stuff in frameset.el.

> Agreed.  I only meant to say that there's no way to do it "cleanly"
> (e.g. such that the arguments to `autoload' are constructed
> automatically from contextual information).

Ah, OK.

As for moving the frameset stuff out of register.el, I'll give it a
bit of thought tonight. I suppose it will be possible to add the arg
to the :jump-func call in a back-compatible way.

   J

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Stefan Monnier]

> As for moving the frameset stuff out of register.el, I'll give it a
> bit of thought tonight. I suppose it will be possible to add the arg
> to the :jump-func call in a back-compatible way.

In the very worst case you could jsut not pass that info the frameset.el
and let frameset.el use current-prefix-arg.  Kind of ugly and bug-prone,
but it should work fine in practice.


        Stefan

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

On Thu, Aug 8, 2013 at 8:22 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> In the very worst case you could jsut not pass that info the frameset.el
> and let frameset.el use current-prefix-arg.  Kind of ugly and bug-prone,
> but it should work fine in practice.

I've chosen that path because I don't see any clean way to fix the
registerv issue (I see some unclean ways, but they're far worse than
using current-prefix-arg).

   J

RE: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Drew Adams]

> > Also, I've added docstrings for frameset-p and all slot accessors, via
> > (put 'frameset-SLOT 'function-documentation "Docstring."). Oh the joy.
> > Please tell me that I just missed a way to define the docstrings in
> > the cl-defstruct declaration...
> 
> No, I don't think there's such a thing as slot-doc-strings ;-)
> Neither in cl-defstruct, nor in Common-Lisp itself.

No, there isn't.  But there should be, in Emacs.

RE: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Drew Adams]

> >> I know we have some offenders, but please try to stick to "foo-p returns
> >> a boolean".
> > Why?  It's a CL-style predicate, which returns a "generalized boolean"
> > (either nil, or anything else).  What is the harm in returning the
> > version?
> 
> I don't really care that it doesn't always return t as non-nil value.
> But its non-nil value should be treated by its docstring (and by
> callers) as a boolean equivalent to t.  Otherwise it's not just
> a "predicate" but an accessor.

Are you OK with it always returning the version numbar as non-nil value?
That's the question, no?

You don't seriously insist that it sometimes return some other non-nil
value (a strawberry tart, for example) for a frameset argument, do you?

Anytime the type predicate returns a true value - any true value, the
argument must be a frameset.  Sure.  But that does not impose any
condition on the particular true value the predicate must return.  Any
non-nil value will do, including the version number.  A version number
is as true as any other non-nil value in Emacs Lisp.  Don't believe it?
Ask `if', `cond', `and', `or',...

I'm with Juanma on this one.  The function is a type predicate AND it
provides the version number for an object of the type.  You can use it
as a predicate (it is one).  And if the object is a frameset then you
can use it to get the version.  100% of the time.

This is Emacs Lisp, not Scheme.  Since its early days Lisp has used
`nil' and `()' for both false and the empty list; and it has used 
everything else for true.  Some have disagreed with this conflation [*],
but it is the case for Emacs Lisp, just as it is for most other Lisps.

I do agree that the doc string needs to make it clear that `frameset-p'
returns non-nil for a frameset and nil for something that is not a
frameset.  But that is already the case, AFAICT.

If you want it to also explicitly point out that a version number is
always non-nil then I don't see the point in that.  But that is the only
information about the Boolean value and the type predicate that is not
explicitly present already.

> >> Also, this doesn't just test whether it's a frameset or not (which is
> >> what one expects from "frameset-p") but it does some sanity checks
> >> as well, something I'd call maybe "frameset-valid-p".
> > It needs to be called `frameset-p' for cl-typep to work.
> 
> You can use the built-in frameset-p (and then remove the :named
> and :type as well, so that the tag becomes internal/hidden, which
> I also find cleaner)).

The built-in `frameset-p' would not test framesetness!  Not as Juanma
wants to define it.  It would test only for a built-in, minimal
defstruct type definition.

`defstruct' was not designed to be used in only that way.  It explicitly
provides for defining a type predicate that does something different
from a default, auto-generated type predicate.  And thank goodness, too.

If this is done the way Juanma has done it then a nil return value by
`frameset-p' says that the object in question is not a frameset even
if it has tag `frameset' and its slots fit the defining defstruct, etc.
It has to be more than that to be a frameset.  What more?  That which is
coded in `frameset-p' and described in the doc string.

There is nothing wrong with such a tighter-than-default type definition.

You can add, as part of the definition of a type `foo', a condition that
an object is of type `foo' only if it is green, can sing bass notes, and
today is Thursday in UTC time, if you want.  Imagine that!  A viewpoint
that says that the same object (as judged by other predicates, at least)
is of type `foo' on Thursday and not of type `foo' on Friday!  Wrt
fooness, it is not the same object on Thursday as on Friday...

As long as the doc clearly supports the frameset defstruct definition,
describing what an object must be like to be a frameset, there is
nothing wrong with having a more-complex-than-default type definition.

So the question then is, do you not feel that the doc describes the
type definition adequately?

> > If you're suggesting that I split the current frameset-p into a
> > strict-boolean-returning frameset-p and a more thorough
> > frameset-valid-p, I can do that, but, what is the gain?
> 
> That's it's more idiomatic?

I don't think so.  What's the proof of that?

[*] See section 3.1: http://www.dreamsongs.com/NewFiles/HOPL2-Uncut.pdf

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Richard Stallman]

        [ To any NSA and FBI agents reading my email: please consider
        [ whether defending the US Constitution against all enemies,
        [ foreign or domestic, requires you to follow Snowden's example.

I don't think there is any virtue in returning t rather than some
other value that provides additional useful information.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call.

Re: [Emacs-diffs] trunk r113747: lisp/frameset.el: Convert `frameset' to vector and add new slots.

[Juanma Barranquero]

> I don't think there is any virtue in returning t rather than some
> other value that provides additional useful information.

Now frameset-p returns a strict boolean value, but frameset-valid-p
still returns
nil, or a version number.

    J