Improving widgets for take/drop #6641

radeusgd · 2023-05-10T21:29:33Z

Pull Request Description

Related to #6410

Important Notes

Updated some Meta methods (needed for error handling):
- Meta.Type now has name and qualified_name.
- Meta.Constructor has declaring_type allowing to get the type that this constructor is associated with.

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.

radeusgd · 2023-05-12T09:12:38Z

Widget shows what the default values will be:

If no value is provided and no default - a more friendly error is displayed:

radeusgd · 2023-05-12T09:29:15Z

Updated after discussion:

radeusgd · 2023-05-12T11:02:27Z

Got the widgets on Text working as well:

btw. the default visualization for empty text is just empty square box - that is a bit confusing - looks as if the visualization has failed.

JaroslavTulach

Is there a justification for the changes in Meta? I don't see it in PR description and the referenced issue seems too broad.

JaroslavTulach · 2023-05-12T11:48:14Z

test/Tests/src/Semantic/Meta_Spec.enso

            cons = case meta_typ of
                Meta.Type.Value _ -> meta_typ.constructors
                _ -> Test.fail "Should be a Meta.Type.Value: " + meta_typ.to_text

            cons.length . should_equal 1
            cons.at 0 . should_be_a Meta.Constructor
            cons . map (x -> x.name) . sort . should_equal [ "Value" ]
+            cons.each ctor->
+                ctor.parent_type . should_equal meta_typ


Why it should be called parent_type? "Parent" is meant for other things: I can say: Any is parent type of Text and other types as well. Or Number is parent type of Integer. But constructor doesn't have a "parent type".

Java is using declaring class.

Ok, I can rename to declaring_type. Does that sound OK?

...me/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetShortTypeNameNode.java

radeusgd · 2023-05-12T12:00:15Z

Is there a justification for the changes in Meta? I don't see it in PR description and the referenced issue seems too broad.

Fair point, I updated the PR description.

The justification is:

I needed parent_type (now declaring_type) to be able to easily check if a provided constructor is associated with one of expected types (when we add a more helpful error message) or not (where we just throw type error).
I needed short_name (now name), because ~~Enso_Type.to_text is as we know (Type.to_text shall invoke instance method #6300) not reliable.~~ Oh I guess I may have forgot it was fixed 😅

jdunkerley

LGTM. Couple of little things.

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso

jdunkerley

LGTM. Couple of little things.

radeusgd · 2023-05-12T13:02:27Z

Added Range:

I used the defaults from Range.new definition.

JaroslavTulach · 2023-05-15T05:26:20Z

I needed parent_type (now declaring_type) to be able to easily check if a provided constructor is associated with one of expected types (when we add a more helpful error message) or not (where we just throw type error).

Is declaring_type the goal or is "more helpful error message" when "throw(ing) type error" the goal? If the latter, shouldn't it rather be addressed by #6682 than extending Meta capabilities?

…z-6260 * develop: Build nightlies every day. (#6681) Force `newDashboard` default on the CI-built packages. (#6680) Verify ascribed types of parameters really exist (#6584) SuggestionBuilder needs to send ascribedType of constructor parameters (#6655) Improvements to the Table visualization. (#6653) Removing flush (#6670) Improving widgets for take/drop (#6641)

radeusgd · 2023-05-15T09:22:45Z

I needed parent_type (now declaring_type) to be able to easily check if a provided constructor is associated with one of expected types (when we add a more helpful error message) or not (where we just throw type error).

Is declaring_type the goal or is "more helpful error message" when "throw(ing) type error" the goal? If the latter, shouldn't it rather be addressed by #6682 than extending Meta capabilities?

Indeed, the latter. I guess that #6682 will be the general solution to this. Although this particular solution actually creates a more helpful message (partially using declaring_type) than a default configuration type check would. The thing is we detect that an 'unapplied constructor' of the correct type has been passed - and instead of issuing a type error we issue an 'Illegal_Argument' saying that some arguments are missing on the constructor.

Perhaps it can be an inspiration for the typechecker as well - it may indeed be very good for UX to detect situations when an unapplied constructor of the correct type is passed and instead of issuing a type error, issuing an error telling the user they are missing some arguments. Do you think we can do it in #6682? I assume it would "just" require adding a special case to the error reporting logic that checks if the unexpected element is a constructor of an expected type and reporting a slightly different error then.

* develop: Build nightlies every day. (#6681) Force `newDashboard` default on the CI-built packages. (#6680) Verify ascribed types of parameters really exist (#6584) SuggestionBuilder needs to send ascribedType of constructor parameters (#6655) Improvements to the Table visualization. (#6653) Removing flush (#6670) Improving widgets for take/drop (#6641) disable authentication and newDashboard by default (#6664) Add COOP+COEP+CORP headers (#6646)

radeusgd added the CI: No changelog needed Do not require a changelog entry for this PR. label May 10, 2023

radeusgd self-assigned this May 10, 2023

radeusgd force-pushed the wip/radeusgd/take-drop-widgets branch from 945da8f to 139cc29 Compare May 11, 2023 20:28

radeusgd force-pushed the wip/radeusgd/take-drop-widgets branch from 139cc29 to 607a455 Compare May 12, 2023 11:03

radeusgd marked this pull request as ready for review May 12, 2023 11:03

radeusgd requested review from jdunkerley, GregoryTravis, 4e6, JaroslavTulach, hubertp and Akirathan as code owners May 12, 2023 11:03

radeusgd mentioned this pull request May 12, 2023

Widgets fail to parse in the IDE if their label or code contains a double quote " #6657

Closed

JaroslavTulach reviewed May 12, 2023

View reviewed changes

radeusgd force-pushed the wip/radeusgd/take-drop-widgets branch from 607a455 to bd90f76 Compare May 12, 2023 12:15

radeusgd requested a review from JaroslavTulach May 12, 2023 12:15

jdunkerley approved these changes May 12, 2023

View reviewed changes

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso Outdated Show resolved Hide resolved

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso Outdated Show resolved Hide resolved

jdunkerley approved these changes May 12, 2023

View reviewed changes

JaroslavTulach approved these changes May 12, 2023

View reviewed changes

GregoryTravis approved these changes May 12, 2023

View reviewed changes

radeusgd added the CI: Ready to merge This PR is eligible for automatic merge label May 12, 2023

radeusgd added 5 commits May 12, 2023 15:50

add defaults, fallback type error

fc0ece8

implement take/drop widget

dd4a363

Add tests for improved error handling

da814a7

Improve widgets

8dd2d22

Improve Meta

b215635

radeusgd added 13 commits May 12, 2023 15:50

fixes

020f036

better error handling

08bb1ba

fix

54cc87e

fixing labels, reverting defaults

1d88d68

update labels and a default

6a9d323

add a test for Text widgets, to understand why they don't display

86d7b86

fix widgets

9b06e6a

change quotes :o

2b5344e

CR

9dab4a4

fix

9cd5f5a

fix a test, add one more test case

17d1e2d

CR

4685232

missing import

af10b38

radeusgd force-pushed the wip/radeusgd/take-drop-widgets branch from aa7a849 to af10b38 Compare May 12, 2023 14:21

mergify bot merged commit f5071a1 into develop May 12, 2023

mergify bot deleted the wip/radeusgd/take-drop-widgets branch May 12, 2023 15:33

enso-bot bot mentioned this pull request May 13, 2023

Broken redefinition of variable inside if-then expression #6211

Open

unfurl-links bot mentioned this pull request May 16, 2023

SuggestionBuilder needs to send ascribedType of constructor parameters #6611

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improving widgets for take/drop #6641

Improving widgets for take/drop #6641

radeusgd commented May 10, 2023 •

edited

Loading

radeusgd commented May 12, 2023

radeusgd commented May 12, 2023

radeusgd commented May 12, 2023

JaroslavTulach left a comment

JaroslavTulach May 12, 2023

radeusgd May 12, 2023

radeusgd commented May 12, 2023 •

edited

Loading

jdunkerley left a comment

jdunkerley left a comment

radeusgd commented May 12, 2023

JaroslavTulach commented May 15, 2023

radeusgd commented May 15, 2023

Improving widgets for take/drop #6641

Improving widgets for take/drop #6641

Conversation

radeusgd commented May 10, 2023 • edited Loading

Pull Request Description

Important Notes

Checklist

radeusgd commented May 12, 2023

radeusgd commented May 12, 2023

radeusgd commented May 12, 2023

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach May 12, 2023

Choose a reason for hiding this comment

radeusgd May 12, 2023

Choose a reason for hiding this comment

radeusgd commented May 12, 2023 • edited Loading

jdunkerley left a comment

Choose a reason for hiding this comment

jdunkerley left a comment

Choose a reason for hiding this comment

radeusgd commented May 12, 2023

JaroslavTulach commented May 15, 2023

radeusgd commented May 15, 2023

radeusgd commented May 10, 2023 •

edited

Loading

radeusgd commented May 12, 2023 •

edited

Loading