Skip to content

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Sep 23, 2025

This PR updates the bundle/ functions. It does four things:

  1. tidies up docstrings
  2. changes a hook name from :dependencies to :postdeps
  3. changes :has-bin-script to :has-bin
  4. add a bundle/add-manpage function

The changes are in four separate commits so that they can be cherry-picked if one or more of them are undesirable.

I think the changes are mostly self-explanatory but the reason to change :has-bin-script to :has-bin is because the file that is added to the binpath is not necessarily a binscript. Or at least, I can’t see any reason why it is necessarily a binscript (in Predoc, I add a file using bundle/add-bin and it is a compiled file).

@sogaiu
Copy link
Contributor

sogaiu commented Sep 23, 2025

I think syspath is a thing, but perhaps binpath and manpath don't necessarily refer to "things" in the same way. That is, there don't appear to be any established corresponding dynamic variables, unlike (the name of) (dyn *syspath*) for syspath.

In the context of reading docstrings without source (e.g. on the core api page where there is no source code visible), I think it might be better to not say binpath or manpath.


A perhaps less relevant remark concerns the use of / within a docstring, e.g.

(string (dyn *syspath*) "/bin")

Although Windows might allow use of / as a file path separator in many contexts, this use within a docstring doesn't seem so good to me.

Possibly instead of text like:

Scripts will be copied to (string (dyn *syspath*) "/bin")

we could say instead something like:

Scripts will be copied to the bin subdirectory of syspath

Side benefit: the suggested alternative actually takes less characters :)

@pyrmont pyrmont force-pushed the feature.bundle-manpages branch from acc6c75 to 1ed9deb Compare September 23, 2025 09:44
@pyrmont
Copy link
Contributor Author

pyrmont commented Sep 23, 2025

I agree with @sogaiu about binpath and manpath. I'm not sure if his wording is the right way to describe it but I couldn't think of a better alternative so I've updated with that. The changes have been force pushed. (I normally prefer to avoid force pushing to PRs but I've done so here to preserve the cherry-picking potential.)

@sogaiu
Copy link
Contributor

sogaiu commented Sep 23, 2025

Re: 3. changes :has-bin-script to :has-bin

I started looking a bit more closely at janet-pm's source and as part of this noticed that both jpm and janet-pm have both declare-bin and declare-binscript.

It seems there is a difference between a bin and a binscript, as the latter supports a :hardcode-syspath construct that seems to lead to tweaking of the script. Incidentally this was touched on briefly in this issue (see point 4):

  1. Fixes existing problem in janet-pm where hardcoded syspath at the top-level is not honored when run as (defn main .... This change adds a copy of the hardcoded paths inside defn main in a binscript, if main exists in the input.

Not sure what the consequences of :has-bin-script are but thought I'd point out the bits above as a factor for consideration 😅

@sogaiu
Copy link
Contributor

sogaiu commented Sep 24, 2025

I've only managed to find a couple of places where :has-bin-script occurs:

The comment from the OP:

the file that is added to the binpath is not necessarily a binscript

seems correct for install-rule, but in the case of install-buffer, that seems to currently be used only for declare-binscript.

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.

2 participants