Re: master 1e3b0f2: Improve doc strings of project.el

[Dmitry Gutov <dgutov@yandex.ru>]

On 12.07.2020 17:47, Eli Zaretskii wrote:

>> If I said something is a bad idea, don't go ahead and ignore me. This is
>> extremely rude.
> 
> This goes both ways.

It never does with you.

If I work on parts of Emacs I'm not responsible for, I make doubly sure 
to check with authors/maintainers.

How about you extend me this courtesy? I'm the author and the maintainer 
of this package.

>> Not documenting internal information is a valid choice.
> 
> It isn't internal.  Anyone who wants to use the return value will need
> to understand its possible form(s).

Yes, it is. Apparently you don't understand how cl-generic works.

>>> But other than that, if someone needs to write code for the VC project
>>> backend, how can they NOT rely on the form of the object that
>>> project-try-vc returns?
>>
>> Nobody should "write code for the VC project backend" except its
>> author(s).
> 
> This is a community project, and everybody is welcome to contribute
> code to any part of Emacs.  Restricting some parts of code to a single
> person is not a good way of ensuring Emacs's future.

Not the person. The place. project-vc lives in project.el.

Internal information can reside there without being documented, if all 
of its producers and consumers also reside there.

>> Whatever fuctionality users need to implement, should target the open
>> API. Which does not depend on the shape of the return value of
>> project-try-vc.
> 
> With respect, I disagree, having read the code and having considered
> how I would implement something related to it.  That is the single
> most important reason why I'm trying to improve the documentation of
> this package: to make it easier for others to expand and extend it.

If you had asked me questions like "how do I implement this or that", 
none of my answers would have included the format of the return value of 
project-try-vc.

So the docstring shouldn't either.

You can still ask those questions.

>>> Why should that form, which is important for
>>> handling any VC project, and is thus part of a public API, be left
>>> undocumented?
>>
>> No, they can't. That information can freely change between versions, so
>> any reliance on it will create compatibility problems when upgrading.
> 
> Then let's say that.  But concealing the information will not solve
> the problem, because you cannot conceal it.  By not mentioning it and
> the caveats to go with it we will be punished twice: we will make it
> harder for people to understand the current code and contribute to it,

There's nothing hard in understanding what the current code returns: you 
navigate to the function's definition and look at the end.

> and we will not make sure they realize that relying on these details
> is not a good idea.

Like I said, if you're not satisfied with how that recommendation is 
conveyed, feel free to add clarifications. *Without* giving the example 
of said values in the docstrings.

>> If my method of saying "don't rely on this format" is insufficient,
>> please go ahead and add some more clarifications on that part. But
>> without documenting this said value format.
> 
> You cannot usefully tell people to not rely on something without
> describing that something.

And yet, somehow, parents around the world manage to tell their children 
not to use swear words without swearing profusely themselves.

> And there's the other disadvantage to
> concealing information: obfuscation of the code.

The code is not the issue.

>>> More generally, please put yourself in the shoes of someone who wants
>>> to extend an existing backend, or add a new backend, and try to read
>>> the documentation through their eyes.
>>
>> If they create a new backend, they will read the code of the existing
>> one.
> 
> That's not how we encourage extending Emacs.  Some minimal
> documentation is needed before we can in good faith tell users to read
> the code, and project.el currently falls short of that minimum, IMO.

You seem to be criticizing something else than what I said.

And I never said we don't need documentation.

>>> They will need to know
>>> something about the methods they should implement/extend, about which
>>> ones are and which ones aren't mandatory.
>>
>> That is orthogonal to this issue.
> 
> I don't think it's orthogonal, I think it's all part of the same
> issue.

Mentioning (vc . "path/to/root") somehow helps the users know which 
methods they need to implement?

You must be kidding.

>>> They will need to know
>>> something about what the project object is or can be, and what minimum
>>> capabilities should it have.
>>
>> Hence the description in my latest commit.
> 
> It was insufficient.  And what we have now, after my changes, is still
> insufficient, IMO.  More work is needed.

Of course. On *different* aspects.

>>>> We have enough trouble with user-defined functions returning (cons 'transient some-root-dir) already.
>>>
>>> Which trouble is that?
>>
>> People try to customize the root-finding logic this way, and as a result
>> get a project backend that behaves more slowly than the VC one. And that
>> affects all of project-* commands that they might like to use.
> 
> People learn by doing and by making mistakes.  It is not a catastrophe
> if their first attempt is suboptimal.  Making the documentation better
> might be one way of helping them understand how to DTRT.

Right.

>>> And as long as we are talking about this: you mentioned the
>>> "transient" project without defining what it is.  That is not useful.
>>> I tried to add some minimal explanation of that, and also fixed
>>> another related omission.
>>
>> I would say thank you, but you again documented the return value there.
> 
> Of course.  It's a value that others need to be aware of.
> 
>> The value is *irrelevant*, and it is, again, internal to the backend.
> 
> Which backend is that?

The 'transient' backend.

Okay, internal to project.el, if we want to be more precise.

 > AFAICT, project-current is backend-agnostic.

Except when we want to return some project instance where none were 
detected.