-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
base: develop
Are you sure you want to change the base?
Conversation
context-aware hinter code
Branch kamakshi
user-defined context-aware variables, functions & classes suggestion
added listOfAllFunctions.json
…estions for user-defined functions
minor bug fix in autocomplete suggestions
…eb-editor into Branch_Kamakshi
resolves all merge conflicts
…name in different scopes
…on name in different contexts
Processing develop
.env.example
Outdated
@@ -19,7 +19,7 @@ MAILGUN_KEY=<your-mailgun-api-key> | |||
ML5_LIBRARY_USERNAME=ml5 | |||
ML5_LIBRARY_EMAIL=examples@ml5js.org | |||
ML5_LIBRARY_PASS=helloml5 | |||
MONGO_URL=mongodb://localhost:27017/p5js-web-editor | |||
MONGO_URL=mongodb://127.0.0.1:27017/p5js-web-editor |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
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. function setup() {
createCanvas(400, 400);
}
function draw() {
background(220);
hello(); // call
} hi.js function hello() { // definition
console.log("Hello");
} Renaming |
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. |
Thank you for your feedback and all your suggestions! I'll also try to add the jump to definition highlight feature. |
Thanks so much for your work on this @kammeows! I think this looks really great so far, and I really love how much smoother this overall looks! I don't think I have too much to add on to @diyaayay's comments, and I feel that they covered most of my initial thoughts while testing out the feature! I do have a quick question—while loading this branch, I ran into a small version mismatch issue between |
Hey @raclim! Thank you for taking a look on this. No I did not encounter any such issue. If it comes up again, we can look into whether we need to adjust the package versions in the repo to prevent others from hitting the same issue. |
This PR is only for review.