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

[Eli Zaretskii]

> Cc: philip@warpmail.net, theo@thornhill.no, emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 13 Jul 2020 04:51:33 +0300
> 
> On 12.07.2020 19:17, Eli Zaretskii wrote:
> 
> > Can we approach this from a different angle, please?  Can you tell why
> > you are so opposed to having this small piece of information in the
> > doc strings?  What harm can it do that you consider unacceptable?
> 
> Let's try this again.

Sadly, you never answered my questions.  I still hope that you do,
because I don't understand the staunch objections, to the degree that
causes extremely unkind responses, to improve the doc string by saying
something that is both simple and correct.

Instead of answering my practical questions, which could have led us
towards some acceptable compromise, you've elected to reiterate your
views, which just makes another round of the same dispute we've been
having, without any hope for reconciling the differences.  Because it
turns out we disagree even more than I thought, on the very principles
behind this, see below.

Please do consider answering my questions.  I think it is important to
do that, so that we will be able to usefully discuss similar issues in
the future.  We disagree about the principles, so the only hope is to
find some practical compromise with which we will be able to live.

> This information is private (or "internal") similar to when we designate 
> variables or function private: we don't want people to rely on it 
> because a) that might lead to wrong decisions, b) that might lead to 
> breakage because this information is subject to changing at will. For 
> example, I'm considering turning the whole value into a list (it's 
> currently a shallow cons) and adding the VC backend symbol to the 
> project-try-vc's return value as the second element to save some CPU cycles.

When the form of the return value changes, we will update the
documentation.  This kind of thing happens all the time in Emacs
development, and is nothing to be afraid of.  Especially if you
consider some information "private", which means we under no
obligation to preserve its form.  We even have an "Internals" chapter
in the ELisp manual (woefully incomplete, but not because we decided
not to tell those missing details).

This is why I'm frankly astonished by your opposition to document the
return value.  Once again: what's the catastrophe in doing that?

> Here are things we want third-party code to be able to do:
> 
> - Consume the public project APIs, which are currently limited to 
> 'project-current' and several generic functions defined for project 
> instance values, to define new commands or supplement existing ones 
> (e.g. to query the current project for its root or its list of files).

Where is this documented?  I don't think I see it.

> - Create new project backends. Note that we don't want *everybody* to do 
> that because creating a well-performing backend is a significant amount 
> of work (unless the user has very particular needs and works with small 
> projects). But we want this to be a solid option, and it would be ideal 
> if, say, a handful of backends become popular someday. To create a 
> backend, one needs to choose the format of its instance value (which 
> shouldn't match any of the existing formats), write its 
> project-find-functions element, and define the project-root override 
> that dispatches on its instance value. Preferably, also implement 
> overrides for project-ignores and project-files.

Where is it documented what needs to be done for creating a new
backend?  Again, I don't think I see it documented.

> Neither of these options requires knowing the exact format of 
> project-try-vc's return value, if only to stay away from it. But the 
> odds of a collision seem very low (and there are ways to reduce them 
> even further).

These are just some of the possible development directions.  There are
others, no less legitimate ones.  In particular, minor improvements to
the code, which don't change the overall design and implementation.
These and others do require to be aware of the form of the return
value, or at least they can be helped by knowing that.

Moreover, I'm quite sure that if you try to document the missing
pieces mentioned above in a way that is useful for potential
contributors, you will find soon enough that the policy of hiding
useful information about interfaces, even "internal" ones, works
against you: it makes it much harder to say something useful about how
the various objects and methods interact or should interact.  You will
eventually be forced to say "read the code" all the time.  Which means
the documentation cannot do one of its important jobs: help to
understand the code.

But more generally, it is not our prerogative to decide for others in
what directions they may or may not take the development in the
future.  This is not our pet project, this is a community project, and
any member of the community is welcome to propose changes to any part
of Emacs.  If the change is useful for the users, and is done cleanly,
we welcome and accept such changes -- this is how Emacs was and is
being developed for 40 years.  It is what brought us where we are
today.

So I very much object to the policy you describe above, which is that
you personally decide up front what kinds of changes are and aren't
legitimate, based on some principles that are not necessarily shared
by others (certainly aren't documented anywhere in the Emacs
contribution-related docs), and then unilaterally implement this rigid
policy, right down to the level of the smallest detail in the doc
strings.  This is an extremely radical and harsh policy, which I don't
think ever existed in Emacs, nor do I think it should exist.  If
accepted, it will IMO harm the Emacs development in the long run.

> Here are, on the other hand, things that people generally shouldn't do, 
> and yet I have seen them done:
> 
> - Extract data from the project instance value directly, instead of 
> going through e.g. 'project-root'. For instance, writing (cdr 
> (project-current t)) instead of (project-root (project-current t)). The 
> former is bad code.
> 
> - Write a new project-find-functions element which returns an instance 
> value that belongs to a built-in backend (usually, the 'transient' one). 
> AFAICT, this is usually done by cargo culting, without understanding of 
> the consequences.

See above: I disagree very much with saying flatly that we don't want
these.  These types of changes are not "verboten" in principle, they
should be each one of them judged on its own right, and if done
cleanly and efficiently, they are not in any way "wrong".

In a Free Software project, you cannot forbid people from trying
different development directions, even dangerous ones, you can only
warn them about the dangers by documenting them.  Which is one more
reason for having good, detailed documentation.  Refraining from
mentioning something because you think it is dangerous is the wrong
way, because people are curious and will find what you are trying to
hide.  When they do, no amount of "I didn't want you to know that, so
don't do that" will help, believe me.

> Both of these require knowing the exact value formats of project 
> instances used by built-in backends. Thus, documenting them will 
> encourage these usages.
> 
> I don't want to prohibit the last option outright, but we shouldn't 
> leave any impression that it's legitimate either. And most problems that 
> people try to solve this way (predominantly: customizing project root 
> markers, AFAIK) should be filed as bug reports/feature requests.

As I already said several times, the right way of doing this without
throwing the baby with the bathwater is to document the value and then
tell this form is subject to change without notice.  It is a very
simple solution that keeps both goals in our sights without losing any
of them.

> > I added that piece of information because without it I couldn't wrap
> > my head around what the code does, in particular when it tries to
> > determine which projects are equal to which.
> 
> But it's the wrong direction.
> 
> I mean, you *can* look at the shape of the instance returned by 
> project-try-vc (which is trivial to do: evaluate the form 
> (project-current t) and look at the echo area)

If the form of a return value can only be obtained by running the
code, it is IMO a clear sign that the documentation fails to do its
job.  Which is exactly why I decided to document it.

> but it doesn't give you any guarantee: after all, we need to make
> sure project-switch-to-buffer's behavior makes sense for different
> backends, even ones we didn't write.

Irrelevant: the information I added was about a single _existing_
backend.  If you have proposals how to say something sensible and
helpful about others, I'm all ears.

> So documenting that one backend's instance format is not a solution.

It is a solution for those who want to understand better how the
_existing_ code works.  Admittedly, it's a small step, but it's better
than nothing, and it's a step in the right direction.

> Instead, it seems we have to document that said value should uniquely 
> identify the returned project and be "stable": not change over time or 
> when fetched from different buffers belonging to the same project. That 
> also means that the value shouldn't contain any caches inside of it.
> 
> And once we add these requirements, the answer to "which projects are 
> equal" translates to:
> 
> - The backend is the same,
> - The instances describe the same project, but the exact fashion depends 
> on the backend. As it would be anyway.

Of course, none of this is described anywhere in project.el.  It isn't
even clear what objects are part of the design, what are the
requirements from each object, how to establish of some arbitrary Lisp
object is "an instance of a project", how to compare them, etc. etc.
In sum, the crucial information that is expected from any object
system is missing, and potential developers are evidently expected to
glean it all by reading the code.  How does that make sense, and how
is it TRT to refuse improvements of the documentation when what we
have is this imperfect situation?

> > I reckon if this was
> > difficult for me, there will be others who could benefit from having
> > that info.
> This kind of internal information might be better suited for code 
> comments.

But it is missing from comments as well.  It is simply not there.

> I'm not sure where you could put this particular bit of
> information, where it would both be relevant and yet not obvious from 
> the nearby code, though.

_Any_ place is better than _no_ place.  "Not sure where to put" is not
the same as "let's not document this".