scribble/srcdoc raised exception fix and class-doc syntax form

[Dominik Pantůček]

Hello,

as a heavy user of scribble/srcdoc I've always missed a defclass provide
form equivalent. Therefore I wanted to implement my own. I decided to
use thing-doc as a starting point and noticed that it raises misleading
message if id is not an identifier. Minimal working example follows.

---- MWE: test.rkt ----
#lang racket
(require scribble/srcdoc)
(provide (thing-doc 5 number? "Just a number."))
---- MWE: test.scrbl ----
#lang scribble/manual
@(require scribble/extract)
@include-extracted["test.rkt"]
----

Running scribble --pdf test.scrbl produces:

test.rkt:4:12: parameter/doc: expected an identifier
at: 5
in: (thing-doc 5 number? "Just a number.")
location...:
test.rkt:4:12
context...:

Looking at the sources file scribble-lib/srcdoc.rkt, the line 580 lists
the wrong exception identifier. A patch against current master follows:

====
diff --git a/scribble-lib/scribble/srcdoc.rkt
b/scribble-lib/scribble/srcdoc.rkt
index ee977a16..e9dd3311 100644
--- a/scribble-lib/scribble/srcdoc.rkt
+++ b/scribble-lib/scribble/srcdoc.rkt
@@ -577,7 +577,7 @@
[(_ id contract desc)
(begin
(unless (identifier? #'id)
- (raise-syntax-error 'parameter/doc
+ (raise-syntax-error 'thing-doc
"expected an identifier"
stx
#'id))
====

Also I implemented a simple defclass wrapper as a provide form named
class-doc:

==== class-doc syntax form ====
(define-provide/doc-transformer class-doc
(lambda (stx)
(syntax-case stx ()
[(_ id super (intf-id ...) pre-flow)
(begin
(unless (identifier? #'id)
(raise-syntax-error 'class-doc
"expected an identifier"
stx
#'id))
(unless (identifier? #'super)
(raise-syntax-error 'class-doc
"expected super class identifier"
stx
#'id))
(values
#'[id any/c] ; contract not used?
#'(defclass id super (intf-id ...) . pre-flow)
#'((only-in scribble/manual defclass))
#'id))])))
====

It does not handle the maybe-link keyword argument as I am not using it
and I didn't dive into where it is used in order to test it.

maybe-link =
| #:link-target? link-target?-expr

I would appreciate any feedback and/or improvement suggestions.

Is there a chance this new provide form can be merged into
scribble-lib/srcdoc.rkt in the future or is it better to create a
separate package for it?


Cheers,
Dominik

[Ben Greenman]

These changes look great. Can you open a pull request for the
racket/scribble repo?

https://github.com/racket/scribble


Some comments below

> Also I implemented a simple defclass wrapper as a provide form named
> class-doc:
>
> ==== class-doc syntax form ====
> (define-provide/doc-transformer class-doc
> (lambda (stx)
> (syntax-case stx ()
> [(_ id super (intf-id ...) pre-flow)
> (begin
> (unless (identifier? #'id)
> (raise-syntax-error 'class-doc
> "expected an identifier"
> stx
> #'id))
> (unless (identifier? #'super)
> (raise-syntax-error 'class-doc
> "expected super class identifier"
> stx
> #'id))
> (values
> #'[id any/c] ; contract not used?
> #'(defclass id super (intf-id ...) . pre-flow)
> #'((only-in scribble/manual defclass))
> #'id))])))
> ====

It looks like:
- `class?` would be a better contract than `any/c`
- maybe `defmethod`, `this-obj', and others should be required, in
case the `pre-flow` needs them

> ....

> It does not handle the maybe-link keyword argument as I am not using it
> and I didn't dive into where it is used in order to test it.

That seems fine because `thing-doc` doesn't provide a maybe-link to
`defthing` either.

[Dominik Pantůček]

Hi,

thank you for the quick response.

On 25. 09. 19 17:11, Ben Greenman wrote:
> These changes look great. Can you open a pull request for the
> racket/scribble repo?
>

opened PR#212.

> It looks like:
> - `class?` would be a better contract than `any/c`

Definitely makes sense, as the result is contract-out. I will change that.

> - maybe `defmethod`, `this-obj', and others should be required, in
> case the `pre-flow` needs them

Adding at least defconstructor and defmethod is definitely a good idea
(those are used heavily within defclass anyway). Although I've rarely
used this-obj so far, it looks like a very good idea indeed, so I will
add these three symbols into require value.

>
>> ....
>> It does not handle the maybe-link keyword argument as I am not using it
>> and I didn't dive into where it is used in order to test it.
>
> That seems fine because `thing-doc` doesn't provide a maybe-link to
> `defthing` either.
>

I see. So let's just forget about it right now then.

I will clean it up and create another PR for it.

Should I include a brief documentation in
scribble-doc/scribblings/scribble/srcdoc.scrbl within the same PR as well?


Cheers,
Dominik

[Ben Greenman]

> Should I include a brief documentation in
> scribble-doc/scribblings/scribble/srcdoc.scrbl within the same PR as well?

Yes!

And if there are tests for scribble/srcdoc, it'd be good to add some
for `class-doc` before merging

[Dominik Pantůček]

On 25. 09. 19 19:32, Ben Greenman wrote:
>> Should I include a brief documentation in
>> scribble-doc/scribblings/scribble/srcdoc.scrbl within the same PR as well?
>
> Yes!

Btw, looking at the struct-doc and struct*-doc descriptions maybe I
should actually provide class-doc and class*-doc forms. The former for
documenting classes without interfaces and the latter with interfaces
list. The underlying defclass omits interfaces in the generated
documentation if the interfaces list is empty (which is something the
transformer can do for class-doc then). This way it would strictly
follow the difference between class and class* forms.

>
> And if there are tests for scribble/srcdoc, it'd be good to add some
> for `class-doc` before merging
>

There are tests only for proc-doc-transformer and
proc-doc/names-transformer. All other forms are without any tests (which
sort of makes sense).


Dominik
P.S.: Btw, I am subscribed to the list ...

[Dominik Pantůček]

Hi,

On 25. 09. 19 19:43, Dominik Pantůček wrote:
>
> On 25. 09. 19 19:32, Ben Greenman wrote:
>>> Should I include a brief documentation in
>>> scribble-doc/scribblings/scribble/srcdoc.scrbl within the same PR as well?
>>
>> Yes!
>
> Btw, looking at the struct-doc and struct*-doc descriptions maybe I
> should actually provide class-doc and class*-doc forms. The former for
> documenting classes without interfaces and the latter with interfaces
> list. The underlying defclass omits interfaces in the generated
> documentation if the interfaces list is empty (which is something the
> transformer can do for class-doc then). This way it would strictly
> follow the difference between class and class* forms.
>

it's there as PR#213.

However, I have no clue what @history[#:added ...] should be attached to
the scribblings. The git tags are the same as for Racket releases and
the version from scribble-lib/info.rkt is 1.29. I can guess 1.30 might
be fine, but I'd rather get this confirmed.


Cheers,
Dominik