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

Support widgets and placeholders in more complex expressions #5656

Merged
merged 15 commits into from
Feb 20, 2023

Conversation

Frizi
Copy link
Contributor

@Frizi Frizi commented Feb 14, 2023

Pull Request Description

Implements #5032

Added support for widgets in infix expressions (right now only used for file paths)
image

Widgets and placeholders are handled for all chained methods within a node
image

The qualified method call convention and static methods no longer confuse the argument placeholders
image

Type constructor expressions now receive placeholder arguments. The placeholders work on nested expressions within a node.
image

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@Frizi Frizi force-pushed the wip/frizi/longer-infix-chain-widgets branch from 9ea001d to 1db772d Compare February 14, 2023 12:37
@Frizi Frizi marked this pull request as ready for review February 14, 2023 15:44
@farmaazon
Copy link
Contributor

  1. Perhaps the engine problem: on screenshot below, I had the node on the right first, and then just added the node on the left; as you see, the drop down does not suggest column names anymore
    Screenshot from 2023-02-17 12-14-10

  2. On the same picture you can see that if I pick some Filter_Condition (on the screenshot Equal) then I cannot change it anymore. Not sure if part of this task though.

  3. Also not sure if part of this task, but if I write Equal by hand (or remove _ placeholder), then I don't see drop downs anymore. Visualization also show nothing.

image

Continuing QA...

@farmaazon
Copy link
Contributor

No other issues found.

@Frizi
Copy link
Contributor Author

Frizi commented Feb 17, 2023

@farmaazon

  1. Yes, this looks to be an engine problem. What you see is that the node on the left started using tag_values (static dropdown data), which means the widget query didn't return a widget. I suspect that happens when that node's expression postion in the source file is below an expression that caused an evaluation exception. In this case, I believe that the filter condition expression causes it. Which engine version are you using?

Also, the trailing _ in the Filter_Condition suggestions has been removed in this PR. You need to build the standard library locally to see that. This is the only non-IDE change.

  1. This looks wrong, and looks to be my fault. I'm going to investigate it further. Looks like expression groups don't create the right span tree.

  2. is essentially a combination of 1. and 2. You don't get the widgets and visualization because there is an error in the graph, and because there is something wrong with handling Group nodes in argument position. Other than that, the inserted to placeholder looks right.

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.

@farmaazon

  1. Yes, this looks to be an engine problem. What you see is that the node on the left started using tag_values (static dropdown data), which means the widget query didn't return a widget. I suspect that happens when that node's expression postion in the source file is below an expression that caused an evaluation exception. In this case, I believe that the filter condition expression causes it. Which engine version are you using?

I use nightly from 2023-02-15

  1. This looks wrong, and looks to be my fault. I'm going to investigate it further. Looks like expression groups don't create the right span tree.

I can confirm this fixed. As other bugs are related to the Engine, the QA is green.

@Frizi Frizi force-pushed the wip/frizi/longer-infix-chain-widgets branch from 3f35ec2 to b01278e Compare February 17, 2023 18:40
@Frizi Frizi added the CI: Ready to merge This PR is eligible for automatic merge label Feb 17, 2023
@Frizi Frizi added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Feb 17, 2023
@Frizi Frizi requested a review from wdanilo February 19, 2023 23:05
@mergify mergify bot merged commit ccdfaca into develop Feb 20, 2023
@mergify mergify bot deleted the wip/frizi/longer-infix-chain-widgets branch February 20, 2023 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants