Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Janet Introduction: Replace (all-bindings) with (doc) for REPL #248

Merged
merged 3 commits into from
Jan 25, 2025

Conversation

27theo
Copy link
Contributor

@27theo 27theo commented Jan 13, 2025

I'm running Janet version 1.37.1-homebrew.

See the introduction page at https://janet-lang.org/docs/index.html:

image

(all-bindings) does not 'print out' all global functions when executed in the REPL, rather it returns them.

$ janet
Janet 1.37.1-homebrew linux/x64/gcc - '(doc)' for help
repl:1:> (all-bindings)
@[% %= * ... yield zero? zipcoll]
repl:2:>

This is already alluded to in the welcome message of the REPL, and the README of janet-lang/janet:

To get a list of all bindings in the default environment, use the (all-bindings) function. You can also use the (doc) macro with no arguments if you are in the REPL to show bound symbols.

The (doc) macro does print out all global functions when executed in the REPL:

$ janet
Janet 1.37.1-homebrew linux/x64/gcc - '(doc)' for help
repl:1:> (doc)


    Bindings:

    % %= * *= *args* *current-file* *debug* *defdyn-prefix* *doc-color*
    *doc-width* *err* *err-color* *executable* *exit* *exit-value*
    *ffi-context* *lint-error* *lint-levels* *lint-warn* *macro-form*
[output truncated by author]

Tiny nitpick, but may save other beginners a bit of confusion.

Thanks for a great doc website for a great language!

@27theo 27theo marked this pull request as ready for review January 13, 2025 09:44
@sogaiu
Copy link
Collaborator

sogaiu commented Jan 13, 2025

I think it's possible the original intent of using the phrase "print out" was to suggest that one would end up seeing something on the screen (so the phrasing might have been intended in a generic sense).

I agree that "print out" does have a technical meaning that is different from "returning a value" (especially in a lispy language) so I can see that the documentation, as it is currently, might lead to some confusion.

Although the current state of the PR may be less confusing, by eliminating the mentioning of all-bindings, does it not miss out on the opportunity to inform the reader of the existence of the all-bindings function?

May be all-bindings can still be mentioned somehow? Perhaps something along the lines of:

Note: see the @code[all-bindings] function for related functionality.

could be added to the end of the proposed changes?

@27theo
Copy link
Contributor Author

27theo commented Jan 13, 2025

I assume that at one point (all-bindings) did print out every binding, and the REPL has been changed since to truncate long lists.

I wouldn't be opposed to retaining a reference to all-bindings, sure.

However, I can't come up with a scenario where a beginner (somebody viewing the 'Introduction' page with installation instructions/hello world) would want to be aware of the all-bindings function. They may have their hands full already with all the others exposed by (doc) and the link to the core API?

@sogaiu
Copy link
Collaborator

sogaiu commented Jan 13, 2025

I assume that at one point (all-bindings) did print out every binding, and the REPL has been changed since to truncate long lists.

TLDR; I did a bit of research into the history of the code and my impression is that all-bindings never did print. There is a longer version of those investigations below if you are interested.

However, I can't come up with a scenario where a beginner (somebody viewing the 'Introduction' page with installation instructions/hello world) would want to be aware of the all-bindings function. They may have their hands full already with all the others exposed by (doc) and the link to the core API?

Having all-bindings suggested right at the time the core API page (and its link) is mentioned seems like a pretty good opportunity for someone to try taking a look at the page with a concrete target to search for. It seems probable that their mind would have just been sensitized to related functionality having had doc's capabilities just explained.

Doesn't that seem worthwhile?


If interested in the longer version...

I think this commit may have been close to where that wording showed up. It seems to have been a modification of docs that used to live in the main language's repository.

I think there was a document named Intoduction.md which got removed in this commit. Looking at the state of the repository for the parent commit, one can see this file where there is the text:

To see a list of all global functions in the repl, type the command

(table/getproto *env*)
# Or
(all-symbols)

Which will print out every built-in global binding (it will not show your global bindings). 

all-symbolics appears to be what all-bindings used to be named -- at least that's what this commit suggests.

At that time, it looks like janet.exe got built out of a main.c which executed init.janet. init.janet in turn ultimately appears to have executed the function repl (that lived in core.janet).

repl in turn invoked run-context. It looks like ordinarily, repl's second argument would have been nil and thus run-context would have had an onsignal argument that would have been set by these lines. It seems that for a non-error evaluation that might have resulted in the use of string/pretty to output results. That's not a function that exists these days, but it does look like it might have had the ability to control how much output would result.

It's true that currently (all-bindings) with default repl settings truncates output (so one sees "..." with things elided). I think that may be because of the default value of the *pretty-format* dynamic variable:

$ janet
Janet 1.37.1-60d9f977 linux/x64/gcc - '(doc)' for help
repl:1:> (dyn *pretty-format*)
"%.20Q"

If that is instead made to be "%.20N":

repl:2:> (setdyn *pretty-format* "%.20N")
"%.20N"

invoking (all-bindings) will not leave out any values.


@27theo
Copy link
Contributor Author

27theo commented Jan 13, 2025

Having all-bindings suggested right at the time the core API page (and its link) is mentioned seems like a pretty good opportunity for someone to try taking a look at the page with a concrete target to search for. It seems probable that their mind would have just been sensitized to related functionality having had doc's capabilities just explained.

Yes, this reasoning sounds good to me. (well put!)

Nice investigation.

I'll replace the reference to (all-bindings) now.

@27theo
Copy link
Contributor Author

27theo commented Jan 18, 2025

@sogaiu Anything else I can do on this?

@sogaiu
Copy link
Collaborator

sogaiu commented Jan 19, 2025

@27theo Athough I comment on things, I think my merging responsibilities extend to the examples directory only.

So to answer your question, I think we wait (^^;

(To be clear, as far as I'm concerned, the PR LGTM in its current state).

@bakpakin bakpakin merged commit f94f2fb into janet-lang:master Jan 25, 2025
1 check passed
@elfenermarcell
Copy link
Contributor

It seems that all-bindings is still referenced on the janet-lang.org homepage:
https://github.com/janet-lang/janet-lang.org/blob/master/content/index.mdz#L78
That should be changed too.

@sogaiu
Copy link
Collaborator

sogaiu commented Jan 28, 2025

@elfenermarcell A PR sort of along similar lines has been made in #277.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants