Broken links to source code in p5.sound reference pages

[rndexe]

Most appropriate sections of the p5.js website?

Reference

What is your operating system?

Linux

Web browser and version

Chrome: 128.0.6613.84 (Official Build) (64-bit)

Actual Behavior

When clicking links on the page here:

Notice any errors or typos? Please let us know. Please feel free to edit src/Amplitude.js and open a pull request!

These files do not exist anymore since they have now been moved to a separate repo.

Expected Behavior

Expected links to go to https://github.com/processing/p5.sound.js

Steps to reproduce

No response

Would you like to work on the issue?

Yes, I can create a pr to make the required changes in ReferenceItemLayout.astro

[ksen0]

Hi @rndexe,

I see the PR you've made for this; I don't see any prior discussion about this change, so please let me know if I missed a conversation about it elsewhere!

In the PR, the link target changes, but the link text stays the same, which is arguably an accessibility issue: https://www.w3.org/WAI/WCAG21/Understanding/link-purpose-in-context.html ("Whenever possible, provide link text that identifies the purpose of the link without needing additional context.")

Additionally, while p5.sound is distinct url than p5.js, the intent with those links is to provide a place where they can open an issue, and I think before making this kind of change, it would be valuable to know if others do expect it to go to a different place for p5.sound. In p5.js, contributors are asked not file a pull request (or start working on code changes) without a corresponding issue or before an issue has been approved for implementation; that is because the proposed fix may not be accepted, need a different approach entirely, or the actual problem is somewhere else.

So I hope that explains changing the label from "Bug" to "Discussion" - the links work as intended, and while a discussion is welcome, some consensus is needed before PR.

[rndexe]

I appreciate the response, however the source code links are currently broken as the p5.sound code is at different location/repo now.

Imo, the source code links are not working as intended.

[ksen0]

thanks for clarifying @rndexe ! I understand more now; when I was looking at the p5.js sound reference it seemed like the lib/addon links were ok, but the src/ links are indeed broken.

Two examples for testing of a fix:

1. https://p5js.org/reference/p5.AudioIn/amp/ has a broken link to https://github.com/processing/p5.js/blob/v1.11.2/src/AudioIn.js#L82
2. https://p5js.org/reference/p5/freqToMidi/ has an alive but maybe unhelpful link to https://github.com/processing/p5.js/blob/v1.11.2/lib/addons/p5.sound.js#L825

Seems like the fix would have to be different for these two cases; from what I understood in the previous PR, it covered (1)?

[rndexe]

I see, I had not considered there'd be different cases. In fact my solution would break the latter case instead.

Would need to understand the basis of this code split into different repos for p5.sound

[rndexe]

On further analysis, it seems that lib/addons links point to a now deprecated version of p5.sound which is included in the p5.js repo. Those functions afaik no longer exist in the new version.

[ksen0]

@rndexe You're right, there's bigger problems here.
Do you have interest in preparing a PR that fixes the links for the existing links in the p5.sound library, and removes deprecated methods? Maybe all those methods and variables that are lib/addon are all deprecated, but it would be very good to check.

Also, based on the fact that p5.sound will remain quite separate, your original approach of having a variable for the URL depending on which library can be used here, it just needs to also be done together with updating some of the individual paths.

To address the link text confusion I was mentioning, what to do think about including the name of the library in the link text? (Something like "Please let us know by opening a ____ issue!")

[rndexe]

I'm not so sure about the removal of deprecated methods from current documentation. People are still out there using the older version, It is still included in the p5 editor by default.
Until there's feature parity, I'd rather have the reference to the old but still usable library till then.

[ksen0]

@rndexe You're absolutely right. Although it's definitely adding confusion that both deprecated and actual methods are shown side by side. Maybe there should be something like a deprecation message on those pages? Maybe that is out of scope.

Updating the summary of this issue below:

1. Any link going to the p5.js documentation should remain unchanged; those under p5.sound starting with lib/addon are part of the p5.sound reference, and maybe could use a deprecation message - but link path should be unchanged
2. Some links should go to p5.sound.js documentation; these are the src/ links

Do you have a sense of how to distinguish these? The lib/addon vs src probably should not be used to make the distinction. Maybe the src/... links should be changed to absolute paths? Could something like that be a potential workaround or do you have other ideas for how to reasonably handle these 2 cases?

[rndexe]

We'll have to see how these reference item page contents are generated, perhaps a flag could be added for the ones generated from lib/addons. (I'm assuming these are auto-generated from code and not handwritten).That flag could then be used in astro to add a deprecation notice. Or maybe that's too complicated.

Taking a step back from the whole issue, I feel the sunsetting of the previous library is still a WIP. These fixes might be premature and become obsolete once the migration is fully complete. It'd make sense to keep this issue open but hold off working on it till the roadmap of the new library is clear.

E.g. the lib/addons folder contains only the older version of p5.sound.js. Once the sunset is complete, it'd be easier to just remove it and the yuidocs config which generates documentation for that path.

[davepagurek]

Just popping in -- I think the deprecated methods from the old lib/addons file showing up might actually be accidental for now.

The reference items are generated by running npm run docs in the source repo. In 2.0, this is done with a new script with some more maintained documentation libraries, and does not include p5.sound. For that, we also generate docs separately in the new p5.sound repo, and combine the results. However, in the 1.x docs that are currently live, the npm run docs script is parsing everything in the p5 directory, which currently includes p5.sound.

So when we switch to the 2.0 branch, the old p5.sound docs will stop showing up. I think the current plan is for a full 2.0 release around February? So it could be worth trying to do a quick fix for the two months in the mean time. My suggestion would be to edit the reference generation script to ignore lib/addons entries, and in the spot that says "Looking for the main p5.js reference?", add an additional link to the old p5.sound archived reference.

[ogbabydiesal]

oop wasn't getting notifications on this thread, subscribed now, let me know if i can modify anything in the p5.sound.js source but looks like we have a plan for pruning some of the old references 🫡

[rndexe]

So to summarize

1. Change reference generation script in p5.js repo to ignore lib/addons
2. Add link to archived reference in p5.sound.js reference
3. Update source code links to p5.sound.js repo
4. Update issues link to p5.sound.js repo but with a more descriptive text

@ksen0 would that require a corresponding issue in p5.js repo as well or can I link to this issue?
I can prepare the PRs if this is the accepted way to move forward.