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

Display placeholders for autoscoped-constructor arguments #9635

Closed
kazcw opened this issue Apr 4, 2024 · 6 comments · Fixed by #9737 or #9916
Closed

Display placeholders for autoscoped-constructor arguments #9635

kazcw opened this issue Apr 4, 2024 · 6 comments · Fixed by #9737 or #9916
Assignees
Labels
-gui d-intermediate Difficulty: some prior knowledge required p-high Should be completed in the next sprint
Milestone

Comments

@kazcw
Copy link
Contributor

kazcw commented Apr 4, 2024

Currently WidgetFunction uses WidgetInput.isFunctionCall to recognize function calls or potential function calls (e.g. Ast.Ident). It should accept Ast.AutoscopedIdentitifier--or maybe we could eliminate this check and use WidgetFunction for any Ast that we have a getMethodCallInfo result for, and let the engine decide what's a function?

This is necessary to fix: #9614 (comment)

@kazcw kazcw added the -gui label Apr 4, 2024
@farmaazon farmaazon added the p-high Should be completed in the next sprint label Apr 8, 2024
@farmaazon farmaazon added this to the Beta Release milestone Apr 8, 2024
@farmaazon farmaazon added the d-intermediate Difficulty: some prior knowledge required label Apr 11, 2024
@farmaazon
Copy link
Contributor

Refinement notes:

  1. The AST "guards" should stay to avoid unnecessary MethodCallIInfo reads. We should just add one more case there.
  2. Extend E2E tests to check if autoscoped constructors works properly (have arguments).

@enso-bot
Copy link

enso-bot bot commented Apr 20, 2024

Ilya Bogdanov reports a new STANDUP for the provided date (2024-04-17):

Progress: Starting the implementation, implementing AutoscopedIdentifier support for AST code. It should be finished by 2024-04-19.

@enso-bot
Copy link

enso-bot bot commented Apr 20, 2024

Ilya Bogdanov reports a new STANDUP for the provided date (2024-04-18):

Progress: Adding e2e test, extending the existing tests for aggregate function was not possible due to weird playwright bug (I couldn’t select another node by clicking on it). Opened a PR. It should be finished by 2024-04-19.

@JaroslavTulach
Copy link
Member

I am trying to verify that everything works, but no luck. I don't think this is fixed. To reproduce my steps apply following diff:

enso.ide$ git diff
diff --git distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso
index fe6731efa2..e10d1ed703 100644
--- distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso
+++ distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso
@@ -23,7 +23,7 @@ make_aggregate_column_selector table display=Display.Always =
     col_names_selector = make_column_name_selector table display=Display.Always add_expression=True
     column_widget = ["column", col_names_selector]
 
-    fqn = Meta.get_qualified_type_name Aggregate_Column
+    fqn = "."
     count = Option "Count" fqn+".Count"
 
     ## Currently can't support nested vector editors so using single picker

rebuild and run the project manager using following command:

enso$ ./run backend sbt -- runProjectManagerDistribution
 INFO ide_ci::program::command: sbtℹ️ Enso Project Manager
 INFO ide_ci::program::command: sbtℹ️ Version:    2024.1.1-dev
 INFO ide_ci::program::command: sbtℹ️ Built with: scala-2.13.11 for GraalVM 21.0.2
 INFO ide_ci::program::command: sbtℹ️ Built from: develop* @ 447f4b5ac6a4e757db41c7e0d3d9252c8153ac7c
 INFO ide_ci::program::command: sbtℹ️ Built on:   Linux (amd64)
 INFO ide_ci::program::command: sbtℹ️ 
 INFO ide_ci::program::command: sbtℹ️ [INFO] [2024-04-29T08:12:10+02:00] [org.enso.projectmanager.boot.ProjectManager$] Started server at 127.0.0.1:30535, press enter to kill server

Once the project manager is started, run the IDE in another terminal

enso$ npm --workspace=enso-gui2 run dev

When I try to use aggregate function, I don't see a choice box on the ..Sum to let me select name of a column:

No column drop down

that's different to what I see when running with the patched distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso file:

Column drop down

With fully qualified name of the constructor, there is a drop down to select a column. Such a drop down isn't visible when working with autoscoped constructor.

I tried to verify this issue multiple times during last week, but the result is always the same. Reopening. @vitvakatu, please try my use-case and sorry, if it works for you.

@enso-bot
Copy link

enso-bot bot commented May 13, 2024

Ilya Bogdanov reports a new 🔴 DELAY for the provided date (2024-05-10):

Summary: There is 21 days delay in implementation of the Display placeholders for autoscoped-constructor arguments (#9635) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: Not really a delay, issue was reopened

@enso-bot
Copy link

enso-bot bot commented May 13, 2024

Ilya Bogdanov reports a new STANDUP for the provided date (2024-05-10):

Progress: Found the cause of regression and prepared a fix, opened a PR. Solving bookclubs. It should be finished by 2024-05-10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui d-intermediate Difficulty: some prior knowledge required p-high Should be completed in the next sprint
Projects
Status: 🟢 Accepted
4 participants