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.

Sign up for GitHub

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

Jump to bottom

Editing outside of the node appears to reshuffle all nodes and break the layout #9389

Closed
1 of 2 tasks
hubertp opened this issue Mar 12, 2024 · 6 comments
Closed
1 of 2 tasks

Editing outside of the node appears to reshuffle all nodes and break the layout #9389

hubertp opened this issue Mar 12, 2024 · 6 comments
Labels
--bug Type: bug -gui d-hard Difficulty: significant prior knowledge required p-medium Should be completed in the next few sprints
Milestone
2024-08 Release

Comments

@hubertp
Copy link
Collaborator

hubertp commented Mar 12, 2024 •
edited
Loading

While waiting on the execution to finish, I start editing some node. Clicking outside of the node appears to collapse all nodes and order them in a single line. Video demonstrates the problem.

Kazam_screencast_00039.webm

There are two problems here

  • GUI should not reshuffle nodes like that (restarting the projects brings the original layout)
  • there are some underlying crashes on the engine side
@hubertp
Copy link
Collaborator Author

hubertp commented Mar 12, 2024

It appears that we end up with () being inserted for the value e.g.

    number6 =
        ()

by GUI? Language server?

@hubertp hubertp mentioned this issue Mar 12, 2024
Merged
2 tasks
@hubertp
Copy link
Collaborator Author

hubertp commented Mar 12, 2024

Steps to reproduce:

  • select the node
  • enter a space
  • click outside of the node
  • all nodes are reshuffled

@hubertp hubertp removed the -compiler label Mar 12, 2024
@enso-bot enso-bot bot mentioned this issue Mar 13, 2024
Closed
mergify bot pushed a commit that referenced this issue Mar 15, 2024
@hubertp
Avoid ArrayIndexOutOfBoundsException with no args (#9393)
3755f90 
The `null` check creates a new Array but always assumed a non-empty one which may lead to
```
java.lang.ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
at org.enso.runtime/org.enso.interpreter.service.ExecutionService$FunctionPointer.collectNotAppliedArguments(ExecutionService.java:778)
at org.enso.runtime/org.enso.interpreter.instrument.job.ProgramExecutionSupport$.sendExpressionUpdate(ProgramExecutionSupport.scala:430)
at org.enso.runtime/org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$executeProgram$3(ProgramExecutionSupport.scala:81)
at org.enso.runtime/org.enso.interpreter.service.ExecutionCallbacks.callOnComputedCallback(ExecutionCallbacks.java:146)
at
org.enso.runtime/org.enso.interpreter.service.ExecutionCallbacks.updateCachedResult(ExecutionCallbacks.java:117
...
```
Added a guard to prevent the exception. The flag will be useless anyway as we won't enter the for-loop in this case.

Appears to be introduced via #8743. Discovered while debugging #9389.
@farmaazon
Copy link
Contributor

Reproduced; As currently numeric input accepts anything, we parse space and put it as "Block" node without lines, and probably then our "AST repairing system" inserts the parenthesis.

I think we should be careful in what is accepted in numeric input - the AST block should not be.

In this issue, undo is also broken sometimes.

@farmaazon farmaazon added d-hard Difficulty: significant prior knowledge required p-medium Should be completed in the next few sprints labels Mar 18, 2024
@farmaazon farmaazon moved this from ❓New to 📤 Backlog in Issues Board Mar 18, 2024
@farmaazon farmaazon added this to the Beta Release milestone Mar 18, 2024
@kazcw
Copy link
Contributor

kazcw commented Mar 18, 2024

This should also be handled differently in Ast.syncToCode--it could reject attempts to sync a non-block into a block, because that isn't safe in general.

@Frizi
Copy link
Contributor

Frizi commented Apr 4, 2024 •
edited
Loading

We need to know what is the expected behavior here. Should the node be deleted, or should such edit be rejected?

@AdRiley AdRiley modified the milestones: Beta Release, 2024-07 Release Jun 15, 2024
@farmaazon farmaazon mentioned this issue Jul 22, 2024
Open
@farmaazon
Copy link
Contributor

Technically, it is fixed by #10457 - now there is no way to put just any code into widgets.

But the problem here may re-appear when implementing Code Input Widget - added a section there about repairing AST to not break graph.

@github-project-automation github-project-automation bot moved this from 📤 Backlog to 🟢 Accepted in Issues Board Jul 22, 2024
@farmaazon farmaazon moved this from 🟢 Accepted to 🗄️ Archived in Issues Board Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
--bug Type: bug -gui d-hard Difficulty: significant prior knowledge required p-medium Should be completed in the next few sprints
Projects
Issues Board
Archived in project
Milestone
2024-08 Release
Development

No branches or pull requests
5 participants
@hubertp @Frizi @kazcw @AdRiley @farmaazon