When picking a Boolean value via the dropdown doesn't recalculate correctly. #6221
Assignees
Labels
--bug
Type: bug
-compiler
d-intermediate
Difficulty: some prior knowledge required
p-high
Should be completed in the next sprint
Milestone
Comments
|
farmaazon
added
d-intermediate
|
it's still valid in the package from 9125568 commit. This issue appear just when user is choosing the value from the drop-down for the first time. |
|
@sylwiabr @jdunkerley: Do you have a full minimal example of this? I just want to double-check that I am looking at the right behaviour, as it seems, the API used in the original video has been changed. |
|
My reproduction with the node Peek.2023-04-23.14-41.mp4 |
Yes, that message indicates the problem is in some builtin. Too bad we don't have a JVM stacktrace. It'd be better if such errors didn't get "swallowed". Is there an option to use in the IDE to run the engine in more verbose mode, @4e6? Or did you copy just the message and not the stacktrace, Michael? Michael, what's your final Enso code? Simple: runs OK for me from CLI and produces |
3 tasks
|
@MichaelMauderer The API is still the same. I can not unfortunately reproduce it with dev console opened. @JaroslavTulach this is happening just in IDE I think. |
Nagranie.z.ekranu.2023-04-24.o.08.25.30.movHere is the video with console opened @MichaelMauderer (starts around 55s - sorry for not cutting it) |
|
Another thought: I am surprised the IDE uses got fixed, but if there is |
|
@JaroslavTulach Sorry this was a false alarm. My dev build was stuck with an old engine (which is also why I saw a different API for I can repro this now, and it seems the IDE is not sending a message to the backend. So investigating this on the IDE side now. |
Probably it was an older engine than 2023.1.1-nightly.2023.3.28 when #6090 got fixed. Good to know this is not an issue. |
|
So, I've traced what happens from the IDE side, and it seems this is an engine issue after all. When looking at the following project
when I choose Which looks fine to me. But after execution ends, we only get an expression update that looks like this Which does not seem quite right as everything in there is Also, when I then remove the which then results in the following expression update Which looks more sensible as it actually contains type information. Unfortunately, I'm not sure how to debug from here, as it appears that something is going when applying the edit on the engine side. Any ideas @JaroslavTulach @4e6 ? |
|
Michael Mauderer reports a new 🔴 DELAY for yesterday (2023-04-26): Summary: There is 2 days delay in implementation of the When picking a Boolean value via the dropdown doesn't recalculate correctly. (#6221) task. Delay Cause: Delayed due to work on other tasks (integration of Execution Environment and Triage) |
|
Michael Mauderer reports a new STANDUP for yesterday (2023-04-26): Progress: Updated PR for #6179, created followup integration PR 6434. Worked on Triage and reproducing issues. Found that #6347 does not resolve this issue. It should be finished by 2023-04-28. Next Day: Next day I will be working on the #6221 task. Finalize previous PRs and get back to investigating this issue. |
|
Michael Mauderer reports a new STANDUP for today (2023-04-27): Progress: Worked on Triage. Continued investigation and traced the issue to (probably) the engine. Updated the task and requested help from the engine team. It should be finished by 2023-04-28. Next Day: Next day I will be working on the #6221 task. Either continue investigation or choose next task if this has to go to the engine team. |
|
This is the verbose log from the LS console when choosing the |
|
And this is the log when switching from “False” to “True” (which results in the correct evaluation) |
|
@JaroslavTulach returning to triage as it seems to be an engine issue. |
|
I don't think it needs to be triage, just re-distributed. |
|
Thanks for the reproducer. I'll start from it early next week. Tuesday follow up: I can reproduce the problem:
I believe this will have something to do with start/end offset of the expression. When the edit adds something at the end of the enso$ git diff
diff --git engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala
index 5445e48fec..ebaa6d8fe3 100644
--- engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala
+++ engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala
@@ -402,10 +402,10 @@ object ChangesetBuilder {
* @return true if the node and edit locations are intersecting
*/
private def intersect(edit: Location, node: Location): Boolean = {
- inside(node.start, edit) ||
- inside(node.end, edit) ||
- inside(edit.start, node) ||
- inside(edit.end, node)
+ inside(node.start - 1, edit) ||
+ inside(node.end + 1, edit) ||
+ inside(edit.start - 1, node) ||
+ inside(edit.end + 1, node)
}
/** Check if the character position index is inside the location.Conclusion: Probably caused by "off-by-one" error introduced when "switching to the new parser" - an adjustment of possitions like in CodeLocationsTest - I believe previously the end of line was part of the |
|
Jaroslav Tulach reports a new 🔴 DELAY for yesterday (2023-05-02): Summary: There is 9 days delay in implementation of the When picking a Boolean value via the dropdown doesn't recalculate correctly. (#6221) task. I've just got assigned to the issue Delay Cause: Restarting work after transfer of the issue Possible solutions: Transfer the issue sooner than the original estimate "time outs" |
|
Jaroslav Tulach reports a new STANDUP for yesterday (2023-05-02): Progress: - investigation of Next Day: Fixing boolean default argument & other bugfixes |
I am not sure anymore it is "off-by-one" error. We are talking about IDE requesting an edit to operator2 = text2.replace text1 text3 (Case_Sensitivity.Insensitive)line trying to change into operator2 = text2.replace text1 text3 (Case_Sensitivity.Insensitive) Boolean.Truethe edit is happening after the closing
I guess the engine should somehow figure out that by adding one more parameter to the line, the value of |
|
Even if the edit does not affect other function arguments, the engine should detect that the |
|
I see there is a |
|
My guess would be that the UUID is updated after the edit. But I don't think it's the issue. Instead, we should make sure that in the scenario, foo a=1 b=1 = a + b
main = foo 1when then Right now we use the |
|
You may wrap the first expression in the brackets to reproduce the issue, i.e. changing |
|
Jaroslav Tulach reports a new STANDUP for yesterday (2023-05-03): Progress: - debugging Next Day: Fixing boolean default argument & other bugfixes
|
|
This is the correct fix: diff --git engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala
index 5445e48fec..1783f1772f 100644
--- engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala
+++ engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala
@@ -341,9 +341,7 @@ object ChangesetBuilder {
if (input.isEmpty) acc
else {
val ir = input.dequeue()
- if (ir.children.isEmpty) {
- Node.fromIr(ir).foreach(acc.add)
- }
+ Node.fromIr(ir).foreach(acc.add)
go(input ++= ir.children, acc)
}
go(mutable.Queue(ir), mutable.TreeSet())the |
2 tasks
|
Jaroslav Tulach reports a new STANDUP for yesterday (2023-05-04): Progress: - defaulted Next Day: Polishing boolean default argument & other bugfixes |
github-project-automation
bot
moved this from 🔧 Implementation
to 🟢 Accepted
in Issues Board
|
Jaroslav Tulach reports a new STANDUP for the last Friday (2023-05-05): Progress: - defaulted Next Day: Bugfixing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2023-04-06_16-52-53.mp4
When I choose a Boolean value from a dropdown it doesn't appear to recalculate correctly.
When I then change the value it then recalculates correctly. And when I change back to the first value it calculates fine.
The text was updated successfully, but these errors were encountered: