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

feat: Provide cwd for che terminal creation #384

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

RomanNikitenko
Copy link
Contributor

@RomanNikitenko RomanNikitenko commented Jul 12, 2024

What does this PR do?

Provides current working directory for a terminal creation when user uses New Terminal (Select a container) action:

  • value of the PROJECTS_ROOT env variable if there is no any workspace folder
  • path to a folder if there is only one workspace folder
  • IDE proposes user to select a folder if there is more then 1 workspace folder

What issues does this PR fix?

eclipse-che/che#23022

How to test this PR?

Empty workspace use case

  • Go to the dashboard => Create Workspace page
  • Use an Editor Definition => Editor Image => set quay.io/che-incubator-pull-requests/che-code:pr-384-amd64
  • Select Empty Workspace to start a workspace
  • main menu => Terminal => New Terminal (Select a container)
  • check that /projects is used as a current working directory: type pwd for example in the created terminal

Workspace contains only one workspace folder

  • Go to the dashboard =>Create Workspace page
  • Use an Editor Definition => Editor Image => set quay.io/che-incubator-pull-requests/che-code:pr-384-amd64
  • Select any sample to start a workspace
  • main menu => Terminal => New Terminal (Select a container)
  • check that project path is used as a current working directory: type pwd for example in the created terminal

Workspace contains more then one workspace folder

  • Go to the dashboard =>Create Workspace page
  • Git repo URL => set https://github.com/RomanNikitenko/web-nodejs-sample/tree/test-two-projects
  • main menu => Terminal => New Terminal (Select a container)
  • Select any folder when IDE proposes it
  • check that selected path is used as a current working directory: type pwd for example in the created terminal

Does this PR contain changes that override default upstream Code-OSS behavior?

  • the PR contains changes in the code folder (you can skip it if your changes are placed in a che extension )
  • the corresponding items were added to the CHANGELOG.md file
  • rules for automatic git rebase were added to the .rebase folder

==========================================
Signed-off-by: Roman Nikitenko rnikiten@redhat.com

rh-pre-commit.version: 2.2.0
rh-pre-commit.check-secrets: ENABLED

Copy link

github-actions bot commented Jul 12, 2024

Click here to review and test in web IDE: Contribute

Copy link

Copy link

@RomanNikitenko RomanNikitenko marked this pull request as ready for review July 15, 2024 12:59
Copy link
Contributor

@vitaliy-guliy vitaliy-guliy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just tested the behavior for the described cases and can confirm the improvement works as expected.

Only one thing I would fix in the next PR is that if you clicked the New Terminal (Select a Container) menu item you are proposed to select a working directory. It is confusing a bit.

So if we have only one container, I would change the menu item title to New Terminal in a Container or New Terminal in a Tooling Container or something like that.

@RomanNikitenko
Copy link
Contributor Author

@vitaliy-guliy
thank you for review and testing my changes!

about

if we have only one container, I would change the menu item title to New Terminal in a Container or New Terminal in a Tooling Container or something like that.

Thanks for your idea for the improvement!
I checked what we can do with it and see that:

  • currently the title is placed on the VS Code browser side
  • info about available containers is available on the che-extension side
  • we should think about localization as well - as we have request for adding it from users

So - currently I don't see some easy way to implement it, probably we could find more suitable name for the command...

Copy link

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>

rh-pre-commit.version: 2.2.0
rh-pre-commit.check-secrets: ENABLED
Copy link

@RomanNikitenko
Copy link
Contributor Author

@azatsarynnyy @vitaliy-guliy

  • I fetched all changes from the main branch to my branch
  • tested different use cases
  • I didn't find any regression - everything works as described in the PR description

So, I'm going to merge the current PR.

@RomanNikitenko RomanNikitenko merged commit 2453ed0 into main Dec 18, 2024
7 checks passed
@RomanNikitenko RomanNikitenko deleted the cwd-for-che-terminal branch December 18, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants