Skip to content

introducing context awareness to the autocomplete hinter in addition to context aware renaming, jump to definition and other refactoring changes (draft work for review) #3594

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

Draft
wants to merge 37 commits into
base: develop
Choose a base branch
from

Conversation

kammeows
Copy link
Contributor

This PR is only for review.

kammeows and others added 30 commits June 5, 2025 17:12
user-defined context-aware variables, functions & classes suggestion
added listOfAllFunctions.json
minor bug fix in autocomplete suggestions
@@ -19,7 +19,7 @@ MAILGUN_KEY=<your-mailgun-api-key>
ML5_LIBRARY_USERNAME=ml5
[email protected]
ML5_LIBRARY_PASS=helloml5
MONGO_URL=mongodb://localhost:27017/p5js-web-editor
MONGO_URL=mongodb://127.0.0.1:27017/p5js-web-editor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this was committed by mistake.

Used to:

- Differentiate between built-in and user-defined functions
- Filter out redefinitions or incorrect hints

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the README as a separate GitHub Gist, and not include it in the final PR to be merged.

@diyaayay
Copy link

I tested a few things, such as the autocomplete for p5-specific functions and the warnings triggered when functions like setup or draw are used in undesired places. They work well.
The Jump to Definition feature also works fine. I tested it across multiple files, and it correctly jumps to the function declaration in another file when the function is called from there. When multiple functions with the same name exist in different files, it follows the runtime behavior. However, instead of just moving the cursor to the function declaration, it would look visually better if the function name were briefly highlighted, for example:
image

But the functionality works well.

@diyaayay
Copy link

The context-aware renaming feature works with Fn + F2 on some systems. I tested it with variable scopes:

function setup() {
  createCanvas(400, 400);
}

var x = 10; 

function draw() {
  var x = 10; // local variable (shadows global x)
  background(220);
  x = 20;     // modifies only the local x
}

I tested context-aware renaming and it works for functions within the same file.
However, when I rename a function in sketch.js that is defined in another file, the function definition in that other file is not renamed.
Example:
sketch.js

function setup() {
  createCanvas(400, 400);
}

function draw() {
  background(220);
  hello(); // call
}

hi.js

function hello() { // definition
  console.log("Hello");
}

Renaming hello() in sketch.js does not update the function definition in hello.js.
Is this the intended behavior?

@kammeows
Copy link
Contributor Author

Regarding context-aware renaming, yes, that’s the expected behavior. From what I’ve seen in most code editors, the renaming only works within a single file. It won’t rename the same function or variable if it’s defined in another file. It would also make the browser editor's functionality more complicated so I thought it would be best to not include such a feature.

@kammeows
Copy link
Contributor Author

Thank you for your feedback and all your suggestions! I'll also try to add the jump to definition highlight feature.

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