[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Dmitry Gutov
Subject: Re: master 1e3b0f2: Improve doc strings of project.el
Date: Sun, 12 Jul 2020 18:08:50 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

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

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

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

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

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

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.


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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]