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

Fix #6521: Main module function calls shouldn't use project namespace #6719

Merged
merged 4 commits into from May 22, 2023

Conversation

Procrat
Copy link
Contributor

@Procrat Procrat commented May 17, 2023

Pull Request Description

Fixes #6521: Picking a function from the CB that's defined in the main module now resolves to Main.<func-name> instead of <project-name>.<func-name>.

Note that, when collapsing nodes to a function, this referral style was already used, so this is just a change in the behaviour of the CB.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the Scala, Java, and Rust style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@Procrat Procrat force-pushed the wip/procrat/fix-func-generation-on-project-rename branch from 876f0bb to f95e306 Compare May 17, 2023 07:57
@Procrat Procrat marked this pull request as ready for review May 17, 2023 07:57
@@ -432,7 +432,7 @@ impl Entry {

fn code_with_static_this(&self) -> String {
if let Some(self_type) = &self.self_type {
format!("{}{}{}", self_type.alias_name(), opr::predefined::ACCESS, self.name)
format!("{}{}{}", self_type.name(), opr::predefined::ACCESS, self.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, it won't work with methods from Main modules of other projects. We need to check if the entry is actually from a currently opened project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I've changed it. Do you want to have another look?

…t-rename

* develop:
  Widgets, Vector as Column, Viz Fixes and Rename Columns (#6768)
  Implement simple variants of `parse` for the Database backend (#6731)
  Enable `require-jsdoc` lint and add two lints related to React (#6403)
  Decimal/Integer .round and .int #6654 (#6743)
  Set suggestion reexports when serializing the library (#6778)
  Fix file uploading node expression. (#6689)
  Using WarningsLibrary to query for warnings (#6751)
  Implement `cast` for Table and Column (#6711)
  Display Initializing project... message when initializing project (#6661)
  Only send suggestions updates when type changes (#6755)
  sbt runEngineDistribution --debug to ease debuggging (#6745)
  Display "modified at" column on the cloud dashboard (#6687)
  Meta.meta Integer . methods (#6740)
  Show spinner while loading directory (#6714)
  Add cloud dashboard to changelog (#6688)
  Fix list editor panics during insertion (#6540)
  Update bug-report.yml
  Remove project create form (#6710)
  Change full-screen visualisation shortcut to shift-space (#6663)
@Procrat Procrat requested a review from farmaazon May 22, 2023 08:07
Copy link
Contributor

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

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

I don't see any changes in required_imports method: please check if we don't add unnecessary import local.Unnamed import when inserting function from Main.

Comment on lines 397 to 398
/// The name of the currently active module.
module_name: Rc<QualifiedName>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a short info why module_name affects the filtering, perhaps with reference to update_matching_info or code_with_static_this.

@Procrat
Copy link
Contributor Author

Procrat commented May 22, 2023

I don't see any changes in required_imports method: please check if we don't add unnecessary import local.Unnamed import when inserting function from Main.

@farmaazon: You were right, it was adding that import. Care to have another look before we move on to QA?

Copy link
Contributor

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

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

Looks good, let's start QA

@MichaelMauderer
Copy link
Contributor

🟢 QA passed

@Procrat Procrat added the CI: Ready to merge This PR is eligible for automatic merge label May 22, 2023
@mergify mergify bot merged commit 9a4b7d1 into develop May 22, 2023
26 of 28 checks passed
@mergify mergify bot deleted the wip/procrat/fix-func-generation-on-project-rename branch May 22, 2023 14:50
Procrat added a commit that referenced this pull request May 23, 2023
* develop:
  Link to new 101 tutorial and not deprecated one. (#6793)
  Add logs section to the bug template (#6798)
  Fix #6521: Main module function calls shouldn't use project namespace (#6719)
  Add project creation time to project metadata (#6780)
  Add a compiler pass to analyze non-existing imported symbols (#6726)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Renaming the project is breaking the use of collapsed function
3 participants