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

fix switch from a detached head to a branch not working #1218

Merged
merged 10 commits into from
Mar 3, 2023

Conversation

basokant
Copy link
Contributor

@basokant basokant commented Feb 7, 2023

Provides fix for #995

If the repository is in detached head (for example after switching to a tag), then switching to a branch will no longer fail. Behaviour after changes:

955-fix.mov

Transforms the fake branch name (HEAD detached at #####) to ##### using Regex.

…ng the fake branch name (HEAD detached at #####) to #####
@github-actions
Copy link

github-actions bot commented Feb 7, 2023

Binder 👈 Launch a binder notebook on branch basokant/jupyterlab-git/fix-issue-955

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

This looks great 🚀

I'm actually wondering if we should rather clean the current branch name for consistency and to avoid other troubles.

The branch is set at

this._currentBranch = data.current_branch;

Would you mind trying applying your fix there to see if it works?

@basokant
Copy link
Contributor Author

basokant commented Feb 8, 2023

This looks great 🚀

I'm actually wondering if we should rather clean the current branch name for consistency and to avoid other troubles.

The branch is set at

this._currentBranch = data.current_branch;

Would you mind trying applying your fix there to see if it works?

Yes I'll try moving the fix there instead 👍 I think that's a good call, I believe this will be more clear to the user on what tag they are checking out.

@fcollonval fcollonval added the bug label Feb 9, 2023
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @basokant

Do you need help to fix the Jest tests?

src/model.ts Outdated Show resolved Hide resolved
…d case can occur when the git repository is checked out at a specific commit that is not a branch head
@basokant basokant requested a review from fcollonval February 13, 2023 20:33
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the tests.

Are you ok with expanding this to add a new flag detached in Git.IBranch at

https://github.com/jupyterlab/jupyterlab-git/blob/master/src/tokens.ts#L816

And use that flag to tune the branch display at

https://github.com/jupyterlab/jupyterlab-git/blob/master/src/components/Toolbar.tsx#L267

@basokant basokant requested a review from fcollonval February 23, 2023 00:39
@basokant
Copy link
Contributor Author

Relocated the translate of the branch toolbar title. Should be ready for a final review @fcollonval

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @basokant

src/components/Toolbar.tsx Outdated Show resolved Hide resolved
@fcollonval fcollonval merged commit 92a9626 into jupyterlab:master Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants