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

Delimited_Format.Delimited encoding option is missing its dropdown #9008

Closed
Tracked by #9007
AdRiley opened this issue Feb 8, 2024 · 6 comments · Fixed by #9042
Closed
Tracked by #9007

Delimited_Format.Delimited encoding option is missing its dropdown #9008

AdRiley opened this issue Feb 8, 2024 · 6 comments · Fixed by #9042
Assignees
Labels
--bug Type: bug -gui d-intermediate Difficulty: some prior knowledge required p-medium Should be completed in the next few sprints
Milestone

Comments

@AdRiley
Copy link
Member

AdRiley commented Feb 8, 2024

When I select Delimited_Format.Delimited and then click on encoding, I expect to get a dropdown of encoding options.

image

Instead there is no dropdown.

delimiter should also have a dropdown.

@AdRiley AdRiley added p-medium Should be completed in the next few sprints -gui labels Feb 8, 2024
@AdRiley AdRiley added this to the Beta Release milestone Feb 8, 2024
@AdRiley AdRiley added the --bug Type: bug label Feb 8, 2024
@farmaazon farmaazon self-assigned this Feb 12, 2024
@vitvakatu
Copy link
Contributor

First of all, let’s check on develop, as my refactoring for dropdowns got landed only today.

@farmaazon
Copy link
Contributor

We miss information from the engine.

This is dynamic configuration we get. The "Delimited" variant has no configuration for parameters:

[["format",{"type":"Widget","constructor":"Single_Choice","values":[{"type":"Choice","constructor":"Option","label":"Auto Detect","value":"Auto_Detect","parameters":[],"icon":""},{"type":"Choice","constructor":"Option","label":"Bytes","value":"Standard.Base.System.File_Format.Bytes","parameters":[],"icon":""},{"type":"Choice","constructor":"Option","label":"JSON","value":"JSON_Format","parameters":[],"icon":""},{"type":"Choice","constructor":"Option","label":"Plain Text","value":"Standard.Base.System.File_Format.Plain_Text_Format.Plain_Text","parameters":[],"icon":""},{"type":"Choice","constructor":"Option","label":"XML","value":"Standard.Base.Data.XML.XML_Format.XML_Format","parameters":[],"icon":""},{"type":"Choice","constructor":"Option","label":"Delimited","value":"Standard.Table.Delimited.Delimited_Format.Delimited_Format.Delimited","parameters":[],"icon":""},{"type":"Choice","constructor":"Option","label":"Excel","value":"Standard.Table.Excel.Excel_Format.Excel_Format.Excel","parameters":[],"icon":""},{"type":"Choice","constructor":"Option","label":"SQLite","value":"Standard.Database.Connection.SQLite_Format.SQLite_Format.For_File","parameters":[],"icon":""}],"label":null,"display":{"type":"Display","constructor":"Always"},"allow_custom":true}]]

This is the suggestion entry we receive. No statics tags here.

            {
                "type": "Add",
                "id": 3093,
                "suggestion": {
                    "type": "constructor",
                    "module": "Standard.Table.Delimited.Delimited_Format",
                    "name": "Delimited",
                    "arguments": [
                        {
                            "name": "delimiter",
                            "reprType": "Standard.Base.Data.Text.Text",
                            "isSuspended": false,
                            "hasDefault": true,
                            "defaultValue": ",",
                            "tagValues": null
                        },
                        {
                            "name": "encoding",
                            "reprType": "Standard.Base.Data.Text.Encoding.Encoding",
                            "isSuspended": false,
                            "hasDefault": true,
                            "defaultValue": "Encoding.utf_8",
                            "tagValues": null
                        },
// ... and much more

Only from those two sources we read drop-down configuration.

@jdunkerley I'm not sure if it's libraries' or engine issue, for now transferring it to you to decide.

@farmaazon farmaazon added -libs Libraries: New libraries to be implemented -language-server triage and removed -gui labels Feb 12, 2024
@farmaazon farmaazon removed their assignment Feb 12, 2024
@jdunkerley
Copy link
Member

We should get the dynamic data for Delimted_Format.Delimited:

    @delimiter make_file_read_delimiter_selector
    @encoding Encoding.default_widget
    Delimited (delimiter:Text=',') (encoding:Encoding=Encoding.utf_8) ...

So we should get a dropdown for the delimiter and for the encoding.

This is as per the behavior of GUI1.

@farmaazon
Copy link
Contributor

farmaazon commented Feb 12, 2024

Ah, I think I know what is going on: we read dynamic config for Data.read, then we assume that it contains config for Delimited (with no parameters information), and don't ask for Delimited itself. This is the only change I could think of which differs from GUI1.

But seemingly, we have to ask for both configurations and merge the parameters information preferring the "parent" widget (Data.read in this case).

@farmaazon farmaazon added the -gui label Feb 12, 2024
@farmaazon farmaazon self-assigned this Feb 12, 2024
@farmaazon farmaazon added d-easy Difficulty: little prior knowledge required d-intermediate Difficulty: some prior knowledge required and removed -libs Libraries: New libraries to be implemented -language-server triage d-easy Difficulty: little prior knowledge required labels Feb 12, 2024
@enso-bot
Copy link

enso-bot bot commented Feb 13, 2024

Adam Obuchowicz reports a new STANDUP for yesterday (2024-02-12):

Progress: Reproduced the issue, and checked communication with the engine. For a moment I thought it's missing widget configuration from their side, but James corrected me - actually, we should ask for dynamic config even if we have some config from "parent widget". Besides, the usual planning and meetings. It should be finished by 2024-02-14.

Next Day: Next day I will be working on the same task. Split some big tasks planned for Q1 so they will be ready for refinement. If time allows, fix the issue with encoding dropdown.

@enso-bot
Copy link

enso-bot bot commented Feb 13, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-02-13):

Progress: Created a fix and pushed as new PR. Besides, some maintenance work: increased retries of e2e test, so any flakiness will be easier to catch (hopefully before merging a PR introducing it). Removed also flaky dependency which was cause of Windows' builds' random failures. It should be finished by 2024-02-14.

Next Day: Next day I will be working on the same task. Plan Dropdowin Widget Improvements, split it into smaller task ready to being refined.

@mergify mergify bot closed this as completed in #9042 Feb 14, 2024
mergify bot pushed a commit that referenced this issue Feb 14, 2024
…9042)

Fixes #9008

Now, even if we inherit FunctionCall config from parent widget (e.g. drop down), we still ask for config of the current call and try to merge them (preferring the inherited parameters).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -gui d-intermediate Difficulty: some prior knowledge required p-medium Should be completed in the next few sprints
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants