Improve documentation on subprocess API

[amano-kenji]

It closes https://github.com/janet-lang/janet/issues/1386

[sogaiu]

I've started going through some of the details and I'd like to mention a few things I noticed initially. What I'm doing is to execute small samples of Janet code as well as compare the prose with what I see in the C implementations. I may also end up tracing execution with gdb / rr at some point.

First, I think there is definitely useful information in this PR and I appreciate the effort of spelling some of these details out. I'm not sure about how much should end up in the docstrings, but I think an initial step is to spell out what should eventually be covered in documentation somewhere (whether that's in the website docs for some things or docstrings for other things).

---

Now for a few specifics...

I have noticed at least one discrepancy so far [1] and I think the text mentions specific Janet function names (e.g. "returns the exit code by calling os/proc-wait") in certain contexts, which strictly speaking is not what actually happens in some cases (e.g. the underlying C implementation is called, not the Janet code).

For this last sort of thing, I think it might be better to say "wait" in prose instead of os/proc-wait as that communicates the intent and:

1. it is more accurate
2. it lessens the burden of updating the docstring in the future should the name of whichever function it is changes (and I think for the most part Janet docstrings avoid mentioning C function names)

As to the discrepancy, for the proposed os/proc-kill docstring, there is text like this:

After sending signal to proc, if wait is truthy, will wait for the process to finish and return the exit code by calling os/proc-wait.

I think this text is referring to this portion of the C implementation:

    /* After killing process we wait on it. */
    if (argc > 1 && janet_truthy(argv[1])) {
#ifdef JANET_EV
        os_proc_wait_impl(proc);
        return janet_wrap_nil();
#else
        return os_proc_wait_impl(proc);
#endif

As I read it, the part of that code that is relevant to the docstring is the lines between #ifdef JANET_EV and #else. This is because AFAIU, Janet docstrings document the behavior of a default build and that means JANET_EV applies (at present anyway). So the lines that appear relevant seem to be:

        os_proc_wait_impl(proc);
        return janet_wrap_nil();

That looks to me like nil is returned. So for a vanilla build, I don't think one will get an exit code.

I think this portion of the docstring may have been from an older time and it may not have been updated when ev got added to Janet.

Anyway, I intend to continue going through the information in this PR, but it's likely to take me some time.

---

[1] I still have qutie a ways to go (^^;

[amano-kenji]

By the way, os/proc-kill actually calls os/proc-wait. It is just the C version of os/proc-wait.

I want to be precise with words. People may get wrong ideas if they just read wait. They may think it's okay to wait twice on the same process because it is not os/proc-wait. I would subconsciously assume that it is not os/proc-wait.

Maybe, os/proc-kill's wait doesn't wait in the background if an error cancels it.

os/proc-wait is not just any kind of wait. It is a specific kind of waiting. Any other kind of waiting will not exhibit the same behaviors as os/proc-wait. For example, os/proc-wait throws an error if os/spawn was called with :x flag and the spawned process exited with non-zero exit code. This behavior will not occur with other kinds of waiting.

If your words aren't as obvious as llama spit, people may subconsciously get wrong ideas... People need a bucket of cold water poured on their body for surprise. Not a whisper they will ignore. Smack them with words. Don't let people guess... in any way.

Consciousness needs loud wake-up calls. Otherwise, wrong subconscious assumptions will creep in.

I changed the documentation. Now, os/proc-kill doesn't mention it returns the exit code if wait is truthy.

[amano-kenji]

I found a little problem. This is os/proc-wait

#ifdef JANET_EV
    os_proc_wait_impl(proc);
    return janet_wrap_nil();
#else
    return os_proc_wait_impl(proc);
#endif

Although ev is enabled, it still returns the exit code. Somehow, janet_wrap_nil returns exit code.....

I just tested this code.

(def proc (os/spawn ["sleep" "10000000"] :p))

(ev/sleep 1)

(printf "exit code %n" (os/proc-kill proc true :term))

The exit code was returned from os/proc-kill although ev was enabled. Thus, I'm going to revert the commit.

Don't guess. Test the code with minimal examples.

[amano-kenji]

To avoid mentioning os/proc-wait in os/proc-kill without causing confusion, janet would need to ship with a document about subprocess management, and subprocess functions will need to refer users to the document for details.

Right now, janet doesn't ship with info janet or anything like that.

Emacs documentation is heavily hyperlinked. I want doc function to refer to relevant documentation as emacs does. Right now, it is os/proc-wait. If you just say wait, there is ambiguity. It is ambiguity that wasted days of my time in quiet desperation.

After wasting days on lack of documentation, insufficient or ambiguous documentation is not acceptable to me.

[sogaiu]

As I've mentioned elsewhere, I think the expectation with Janet is to read the website docs along with the docstrings (though the docstrings live among the website docs as well (for most releases)).

I'll give a couple of examples of things that aren't really in the docstrings nor does it seem practical to put them in some docstrings. How long-strings work has no obvious place in docstrings, nor does syntax for buffers, etc. If we start to think about it, we may come up with other sorts of examples as well, but I'm not going to go into that at this time. A point of this paragraph is that docstrings are not practical as the sole method of understanding a programming language. Having something else as well is way more helpful than not.

The website is available to many users over the Internet and it also exists in a repository that can be cloned if one wants to view those docs locally [1].

Thus I disagree with:

To avoid mentioning os/proc-wait in os/proc-kill without causing confusion, janet would need to ship with a document about subprocess management, and subprocess functions will need to refer users to the document for details.

I believe we're in the process of working on subprocess management docs for the website. I think it would be better to figure out what goes there before settling on what goes in this PR.

Having said that, I think clarifying the information in this PR could be helpful in formulating subprocess management documentation.

---

[1] Possibly that's a bit more challenging for Windows folks because of the use of make, though I think it may still be possible with a bit of work. If the current state is a significant issue, perhaps a PR to try to address that would be accepted.

[amano-kenji]

The fact that janet API doesn't document things thoroughly doesn't mean it's the right way. The fact that something has been done in a certain way doesn't mean it can't be improved.

Emacs has complete hyperlinked documentation online and offline.

If I look up function documentation in emacs, I get hyperlinks to relevant details. If I press Ctrl+h K, then I am transferred to info manual section on the function.

Whatever we end up doing, users reading (doc os/xxx) should be referred to relevant documentation. Referring users to online documentation is not great.

Just mentioning wait in os/proc-close and os/proc-kill is not good enough.

Documentation should refer users to relevant details for accelerated learning.

[sogaiu]

I agree I was mistaken about os/proc-wait.

However:

Somehow, janet_wrap_nil returns exit code.....

This is also incorrect.

What appears to happen is that os_proc_wait_impl(proc); does not end up returning in this case, it does not appear to get beyond this line:

    janet_await();

This was verified using rr's replay ability with a captured session using the code you provided in this comment.

I think it would be better if you read what I wrote more carefully though.

I wrote above:

What I'm doing is to execute small samples of Janet code as well as compare the prose with what I see in the C implementations. I may also end up tracing execution with gdb / rr at some point.

In this case I had not yet traced execution yet due to time constraints, but I think the os/proc-wait discussion shows that I should change that "may" to a "will". It's good that you caught the mistaken conclusion. Thanks.

At present I intend to go through the docstrings in this PR with the slightly amended (s/may/will/) plan in hand to understand them well enough to inform drafting a page on subprocess management. Once the page on subprocess management has taken shape I think it makes sense to revisit the docstrings, possibly to be followed with some iterating over both.

[amano-kenji]

If janet ships with something like info manual, then (doc os/proc-kill) and (doc os/proc-close) can refer people to the info manual section on subprocess management.

Offline documentation should be as complete as humanly possible. Emacs has pioneered the better way to document.

[sogaiu]

The fact that janet API doesn't document things thoroughly doesn't mean it's the right way. The fact that something has bone done in a certain way doesn't mean it can't be improved.

I don't disagree with this, but I don't think there is anyone who is stepping up to do this yet (but see below as well regarding matinenance cost).

I am working with things as they are, I likely won't object to adjusting if things change, but I doubt they will change any time soon.

It's not soley a matter of making the change either, the new arrangement should also match the needs of the main maintainer. It has to be doable for him to look after in the long term because the rest of us come and go.

[sogaiu]

Anyway, I think this part of the discussion is off of the original topic. I am willing to work on improving subprocess management related documentation (both a page for the website and then subsequently related docstrings).

If you want to work on:

Offline documentation should be as complete as humanly possible.

you are welcome to propose this in a separate issue. I think it's likely that a solution will need to consider what the maintenance cost might be, so I suggest any such proposal attempt to consider this point carefully.

[amano-kenji]

Even if the website has a section on subprocess management, there is no guarantee that people will read it or start there.

Many people may start using functions directly. Then, I can imagine them coming to janet matrix channel to ask about os/proc-wait and os/proc-kill because (doc os/proc-kill) doesn't mention peculiarities of os/proc-wait such as not being able to wait twice on a process and os/proc-wait finishing in the background even if it is cancelled by ev/with-deadline.

Because I didn't know that os/proc-wait finishes in the background after it is cancelled by ev/with-deadline, I had to bang my head on the wall for hours.

I had to reverse-engineer those details for hours.... If I'm stuck in my rut after starting with os/spawn, it's very likely that the subprocess section on the website will not occur to my mind. So, even if the section is on the website, I'm still likely to bang my head on the wall for hours and be stressed out.

That's what usually has happened to me even if there is documentation because janet internal doc doesn't refer people to background information.

So, janet internal doc needs to refer people to relevant documentation. Hyperlinks make documentation cohesive. If you skip referring people to documentation, then documentation loses cohesiveness.

Documents need to link to each other. Just documenting something without references or links is not good enough.

If someone has to reverse-engineer something or wonder where the relevant details are, then the documentation is not good enough. Emacs solved that issue with hyperlinked documentation. Adding doc reference is important.

Each wikipedia article shows you a list of relevant articles. Hyperlinking is the lesson of the internet. We should learn from it.

So, my pull reqeust does its best in the absence of extensive offline documentation infrastructure for janet by mentioning os/proc-wait multiple times in people's face. os/proc-wait, os/proc-wait, os/proc-close, os/proc-kill, os/proc-wait, os/proc-wait, os/proc-wait, .... Repeat. Repeat until you are annoyed enough that you read (doc os/proc-wait). Repeating references multiple times is my secret sauce.

I know how a human mind works.

My pull request doesn't increase maintenance load, either because the doc string will be modified when the function is changed.

[amano-kenji]

My rough idea is that janet can ship with offline documentation and (doc os/proc-kill) can mention

Read (help subprocess-management) for details.

or

function doc string ... ... Background information:

• (help subprocess-management)
• ...

The help function can show offline documentation in terminal emulator.

The online documentation is built from the offline documentation, so they are the same.

[sogaiu]

As I wrote above, I suggest you put thoughts about offline documentation in another issue.

As bakpakin has mentioned elsewhere, this is not a chatroom. It is an issue tracker.

Please try to stay on the topic for each issue. Not doing so too much makes it difficult for current readers but also future readers who may be trying to investigate an issue or understand the context of some decision. You are causing clutter that may adversely affect the time, energy, and resources of people trying to improve Janet (which I also believe you are genuinely trying to do).

[amano-kenji]

Okay.

[sogaiu]

Thank you for your cooperation, I appreciate it!

[sogaiu]

For reference, I've started a repository to try to sort out various subprocess bits here.

My current plan (revised from this comment) is to:

• develop my understanding sufficiently as I refine the content there by reading the source and docstrings, writing and running sample code, and examine code execution using gdb / rr
• draft a subprocess page for the website docs
• revisit the docstrings for at least those in this PR (which ATM includes: os/execute, os/spawn, os/proc-wait, os/proc-close, and os/proc-kill) -- FWIW, some work toward this exists in the repository mentioned above, but work is still needed

Note that these things may not happen in exactly the order above, it's just roughly how I expect things to proceed ATM.