Add the ability to rename a project from inside the graph editor

AdRiley commented 3 months ago

Engine team believe all works on their side

farmaazon commented 3 months ago

It seems we may use refactoring/renameProject endpoint in Language Server (not project/rename from PM, because it didn't work well in the old GUI)

farmaazon commented 3 months ago

@AdRiley or @jdunkerley what interaction should start editing the project name? The click already navigates the breadcrumbs. Should it be Alt/Ctrl + click, or double click?

AdRiley commented 3 months ago

Both of those feel hidden. What do we think of a small rename icon? Or if you are already "home" then clicking allows rename?

farmaazon commented 3 months ago

I can start with rename icon, it's most obvious.

farmaazon commented 3 months ago

So I've made an initial implementation on branch wip/farmaazon/renaming-project-in-gui where I use refactoring/renameProject.

The exchange is a bit strange: First, GUI sends the request:

{
  "jsonrpc": "2.0",
  "id": "10",
  "method": "refactoring/renameProject",
  "params": {
    "namespace": "local",
    "oldName": "New Project 16",
    "newName": "Test Project"
  }
}

And these are engine's responses:

{
  "jsonrpc": "2.0",
  "method": "search/suggestionsDatabaseUpdates",
  "params": {
    "updates": [
      {
        "type": "Remove",
        "id": 5893
      },
      {
        "type": "Remove",
        "id": 5894
      },
      {
        "type": "Remove",
        "id": 5895
      }
    ],
    "currentVersion": 10
  }
}

{
  "jsonrpc": "2.0",
  "id": "10",
  "result": null
}

So far, so good.

{
  "jsonrpc": "2.0",
  "method": "refactoring/projectRenamed",
  "params": {
    "oldNormalizedName": "NewProject16",
    "newNormalizedName": "NewProject16",
    "newName": "New Project 16"
  }
}

But here we've received an "update" with new name being actually an old name.

{
  "jsonrpc": "2.0",
  "method": "executionContext/executionStatus",
  "params": {
    "contextId": "45ea8148-8261-4a4c-bea3-3837258d7ef0",
    "diagnostics": [
      {
        "kind": "Warning",
        "message": "Unused variable operator1.",
        "path": {
          "rootId": "edd403b9-2914-4e3f-9551-9b2ace023431",
          "segments": [
            "src",
            "Main.enso"
          ]
        },
        "location": {
          "start": {
            "line": 8,
            "character": 4
          },
          "end": {
            "line": 8,
            "character": 13
          }
        },
        "expressionId": "8eeb41a3-938f-43a5-bfa9-c5d211b5b08f",
        "stack": []
      }
    ]
  }
}

{
  "jsonrpc": "2.0",
  "method": "search/suggestionsDatabaseUpdates",
  "params": {
    "updates": [
      {
        "type": "Add",
        "id": 5896,
        "suggestion": {
          "type": "module",
          "reexport": null,
          "module": "local.NewProject16.Main",
          "documentation": null,
          "reexports": []
        }
      },
      {
        "type": "Add",
        "id": 5897,
        "suggestion": {
          "type": "method",
          "externalId": "a0237571-5ace-4c4a-975e-f15fe9556f8a",
          "module": "local.NewProject16.Main",
          "name": "main",
          "arguments": [],
          "selfType": "local.NewProject16.Main",
          "returnType": "Standard.Base.Any.Any",
          "isStatic": true,
          "annotations": [],
          "reexports": []
        }
      },
      {
        "type": "Modify",
        "id": 5897,
        "reexport": {
          "tag": "Set",
          "value": "local.NewProject16.Main"
        }
      }
    ],
    "currentVersion": 12
  }
}

Here we see the entries with old name back.

{
  "jsonrpc": "2.0",
  "method": "executionContext/executionFailed",
  "params": {
    "contextId": "45ea8148-8261-4a4c-bea3-3837258d7ef0",
    "message": "Module local.Test Project.Main not found.",
    "path": null
  }
}

But here the Test Project is used, which is an actual new name, but wrong version of it (it should be normalized).

4e6 commented 3 months ago

Yes, looks like an issue with visible name/normalize name usage. I'll look into it

enso-bot[bot] commented 3 months ago

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

Progress: Implemented the feature using "rename" button - perhaps still some styling is needed. Tested with engine, and reported all problems I have. It should be finished by 2024-06-10.

Next Day: Next day I will be working on the same task. Work with Dmitry to fix all engine issues.

4e6 commented 3 months ago

About the second issue with calling the refactoring/renameProject directly. The current logic implies that to rename the project you should call the project/rename method of the project manager. The PM updates the package definition and then notifies LS about it by calling refactoring/renameProject, and then LS updates its internal state.

I think it is possible to update the refactoring/renameProject method so that it is self-sufficient. But there may be issues:

if the project is not running, the dashboard calls the project/rename and it only updates the project directory (OK)
if the project is running, the gui calls the refactoring/renameProject of the LS directly. In this case, the language server process needs to rename the directory. It can only be done on the process shutdown. While it is possible, it is less reliable than renaming the project from the project manager
if the project is running, and the dashboard calls the project/rename, then the PM should only notify the LS and it will try to do everything (also, renaming the directory on the process shutdown)

TLDR, if I didn't miss anything, it should be possible to make the refactoring/renameProject work without the need to call the PM first.

enso-bot[bot] commented 2 months ago

Adam Obuchowicz reports a new STANDUP for the last Friday (2024-06-07):

Progress: Checked fixes from the engine, but it does not work properly still. Discussed how to handle those issues - as it turns out, I need to use PM endpoint for renaming project. Read a bit about lexical. It should be finished by 2024-06-10.

Next Day: Next day I will be working on the same task. implement the renaming as discussed with Dmitry.

farmaazon commented 2 months ago

TLDR, if I didn't miss anything, it should be possible to make the refactoring/renameProject work without the need to call the PM first.

I think this should be eventually done, but because I can quite easily (I hope) implement calling PM for renaming project, this is not high priority.

enso-bot[bot] commented 2 months ago

Adam Obuchowicz reports a new 🔴 DELAY for today (2024-06-10):

Summary: There is 4 days delay in implementation of the Add the ability to rename a project from inside the graph editor (#10073) task. It will cause 4 days delay for the delivery of this weekly plan.

Delay Cause: I discovered how to use engine's API only in action. Also I touched the dashboard code, so I expect a bit longer review on that.

enso-bot[bot] commented 2 months ago

Adam Obuchowicz reports a new STANDUP for today (2024-06-10):

Progress: Did a simple implementation of calling PM endpoint for renaming project. There was an issue with our representation of execution context. Implemented an update, but still there are some issues. It should be finished by 2024-06-14.

Next Day: Next day I will be working on the same task. Fix issues, and hopefully make a PR.

enso-org / enso

Add the ability to rename a project from inside the graph editor #10073