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

Add example for spork/argparse/argparse #280

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

brendanhowell
Copy link
Contributor

I am a janet noob so please let me know if I should change anything style-wise.

@sogaiu
Copy link
Collaborator

sogaiu commented Feb 1, 2025

Thanks for your contribution.

I've tried the code and it seems to work 👍

Below are some general concerns that this PR has surfaced -- I don't think they are points about the content of this PR.


IIUC, this is the first instance of examples for spork and I'm not sure how we want to handle them. Some concerns might be:

  • Do we really want spork examples to live in the same directory as those for janet itself?
  • Do we want the examples to live in the janet-lang.org repository? May be the answer is "yes".
  • This PR may result in some changes to how things are arranged. Are there any factors we should consider related to that?

One thing I noticed is that although the examples show up on the overall spork page at:

http://localhost:8000/api/spork/index.html#spork/argparse/argparse

they don't seem to show up on the page for spork/argparse at the appropriate spot:

http://localhost:8000/api/spork/argparse.html

Update: URLs were originally swapped in error, but should now be correct.

Note that a similar thing does not appear to hold for some janet core items. For example, string/split appears on both the overall page for core items as well as on the String Module page.

@brendanhowell
Copy link
Contributor Author

IIUC, this is the first instance of examples for spork and I'm not sure how we want to handle them. Some concerns might be:

* Do we really want spork examples to live in the same directory as those for janet itself?

* Do we want the examples to live in the janet-lang.org repository?  May be the answer is "yes".

* [This PR](https://github.com/janet-lang/janet-lang.org/pull/230) may result in some changes to how things are arranged.  Are there any factors we should consider related to that?

Hmm… I was wondering some of the same things. The proposed PR sounds quite sensible to me. But it looks like it needs a little work. I'm happy to move my example to fit the new organization. Maybe we can leave this open and I will adjust once things get merged.

One thing I noticed is that although the examples show up on the overall spork page at:

http://localhost:8000/api/spork/argparse.html

they don't seem to show up on the page for spork/argparse at the appropriate spot:

http://localhost:8000/api/spork/index.html#spork/argparse/argparse

Note that a similar thing does not appear to hold for some janet core items. For example, string/split appears on both the overall page for core items as well as on the String Module page.

I now see that I actually have the opposite problem but maybe that's what you meant. I can see my example on the overall spork page but not on the sub-page for argparse. Should I file a bug or just wait to see how things look after the re-organization?

@sogaiu
Copy link
Collaborator

sogaiu commented Feb 1, 2025

Maybe we can leave this open and I will adjust once things get merged.

Why don't we go with this for the moment.

I now see that I actually have the opposite problem but maybe that's what you meant. I can see my example on the overall spork page but not on the sub-page for argparse.

Sorry I got the URLs mixed up (^^;

I haved edited my post above to fix it.

Should I file a bug or just wait to see how things look after the re-organization?

If you don't mind doing it please file an issue. I think having an issue for it separately might make it easier to track as well as adjust for if necessary since it's unknown when the re-organization will take place.

@brendanhowell
Copy link
Contributor Author

Relevant bug report #282

@bakpakin bakpakin merged commit 8aed1b1 into janet-lang:master Feb 4, 2025
1 check passed
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.

3 participants