Skip to content

friendly error system preferences item #417

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

Closed
lmccart opened this issue Aug 15, 2017 · 18 comments
Closed

friendly error system preferences item #417

lmccart opened this issue Aug 15, 2017 · 18 comments

Comments

@lmccart
Copy link
Member

lmccart commented Aug 15, 2017

@almchung has implemented the "friendly error system" in the p5.js, that is, functionality that will (optionally) check the parameters passed into p5 functions when they're called and send helpful messages to console. we're about to merge it this week. this feature can be toggled on and off by setting a boolean variable (p5.disableFriendlyErrors = true/false;) in the sketch. is it possible to add this as a preferences item that can be toggled from there, too?

if it sounds ok, I can work on implementing this.

@lmccart
Copy link
Member Author

lmccart commented Aug 15, 2017

I guess one point for debate --
only the p5.js library has this functionality included in it, p5.min.js doesn't. if the user updated the index.html file to link to p5.min.js, the preferences item would no longer have an effect. would this be ok? or would we need to do some detection and auto-hide the item? or is this a reason it shouldn't be in preferences at all?

@lmccart lmccart changed the title friendly error system settings item friendly error system preferences item Aug 15, 2017
@catarak
Copy link
Member

catarak commented Aug 28, 2017

I think it would make sense to, by default, add the non-minified p5.js to a project and include the friendly errors in preferences. Maybe the preference could be removed or unable to be changed if the non-minified version of p5 isn't included in the project?

@catarak
Copy link
Member

catarak commented Feb 1, 2018

@lmccart for this to work as expected, do p5.dom.js and p5.sound.js need to be included as the non-minified versions as well?

@lmccart
Copy link
Member Author

lmccart commented Feb 1, 2018

no, just p5.js.

@catarak
Copy link
Member

catarak commented Feb 1, 2018

i'm having trouble disabling the friendly errors using p5.disableFriendlyErrors = true. does this need to be called in a specific place?

@lmccart
Copy link
Member Author

lmccart commented Feb 5, 2018

using version 0.6.0, both of these are working for me:

p5.disableFriendlyErrors = true;

function setup() {
  ellipse(3, 3);
}
function setup() {
  p5.disableFriendlyErrors = true;
  ellipse(3, 3);
}

@catarak catarak added this to the Public Release milestone Feb 20, 2018
@himanshuc3
Copy link
Contributor

Is this feature already implemented in the web editor?

@catarak
Copy link
Member

catarak commented Mar 1, 2018

it is not! there's not a design for it but it can look similar to the "autosave" option. it should be under the tab "general settings".

@dhowe
Copy link

dhowe commented Nov 2, 2018

I really can't imagine why we're not using the friendly error mechanism by default in the online editor (including p5.js rather than p5.min.js). This seems the place where it would be most useful of all (perhaps with an option to disable for the few advanced users still using the editor who happen to need higher performance)? Multiple times per week I see sketches like below fail silently for new users and it is totally confusing for them:

var size = 300;
function setup() {
  createCanvas(400, 400);
  ellipse(200,200,size);
}

@catarak
Copy link
Member

catarak commented Nov 2, 2018

@dhowe i agree, it should be on by default! i just haven't gotten a chance to work on this issue. i've put the stuff i want to prioritize in a milestone, and this issue is in there.

@dhowe
Copy link

dhowe commented Nov 12, 2018

How about just defaulting to the non-minified p5.js for now? I think it is this line that would need to be changed

@lmccart
Copy link
Member Author

lmccart commented Nov 12, 2018

@dhowe I think one concern was that this significantly slows things down performance-wise and might give a slow first impression of the library. but I may be misremembering.

@dhowe
Copy link

dhowe commented Dec 17, 2018

when new students don't even get basic error messages on mistakes, it is sort of beside the point whether performance is good or not, as they will quickly give up or move on to another tool

this seems absolutely critical from my (teaching) perspective... @catarak ?

@catarak
Copy link
Member

catarak commented Dec 17, 2018

i hear you @dhowe. i agree this should be added soon!

update: got a chance to work on this! needs a little more testing and handling of edge cases.

catarak added a commit that referenced this issue Dec 21, 2018
catarak added a commit that referenced this issue Jan 25, 2019
@catarak
Copy link
Member

catarak commented May 23, 2019

i'm going through a backlog of pull requests and just reached my initial version to add this as a preference. i'm thinking that it's too confusing to add this as a preference, as it's unclear what it should do. for example, is this a per-sketch preference or per-account? maybe, for all projects, non-minified p5.js should be included by default, and advanced users can change to p5.min.js if they need to. this could also fit into a #4 library management system, in which a user could, with a GUI, switch from the non-minified to the minified for a project.

@dhowe
Copy link

dhowe commented May 23, 2019

maybe, for all projects, non-minified p5.js should be included by default, and advanced users can change to p5.min.js

This is what I was thinking -- ideally one could easily switch to minified via the UI (rather than hand-editing the index.html)

@catarak
Copy link
Member

catarak commented May 23, 2019

agreed! the library management feature has unfortunately been pushed off for a while, but i actually don't think it's a ton of work. would be a great self-contained project within the editor for someone to work on!

@lmccart
Copy link
Member Author

lmccart commented May 24, 2019

@catarak makes sense to me!

catarak added a commit that referenced this issue May 24, 2019
catarak added a commit that referenced this issue May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants