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

[Dmitry Gutov]

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.