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

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

Closed
AdRiley opened this issue May 24, 2024 · 12 comments · Fixed by #10243
Closed

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

AdRiley opened this issue May 24, 2024 · 12 comments · Fixed by #10243
Assignees
Labels
-gui x-new-feature Type: new feature request
Milestone

Comments

@AdRiley
Copy link
Member

AdRiley commented May 24, 2024

Engine team believe all works on their side

@AdRiley AdRiley added x-new-feature Type: new feature request -gui labels May 24, 2024
@AdRiley AdRiley added this to the Beta Release milestone May 24, 2024
@kazcw kazcw assigned kazcw and unassigned kazcw May 24, 2024
@farmaazon
Copy link
Contributor

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 farmaazon self-assigned this Jun 5, 2024
@farmaazon
Copy link
Contributor

@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
Copy link
Member Author

AdRiley commented Jun 5, 2024

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
Copy link
Contributor

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

@farmaazon
Copy link
Contributor

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
Copy link
Contributor

4e6 commented Jun 6, 2024

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

@enso-bot
Copy link

enso-bot bot commented Jun 7, 2024

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.

mergify bot pushed a commit that referenced this issue Jun 7, 2024
…rs (#10204)

related #10073

Changelog:
- fix: rename the project to the name containing unsupported characters
@4e6
Copy link
Contributor

4e6 commented Jun 7, 2024

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
Copy link

enso-bot bot commented Jun 10, 2024

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
Copy link
Contributor

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
Copy link

enso-bot bot commented Jun 10, 2024

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
Copy link

enso-bot bot commented Jun 10, 2024

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.

@mergify mergify bot closed this as completed in #10243 Jun 21, 2024
@mergify mergify bot closed this as completed in 4164277 Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui x-new-feature Type: new feature request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants