Checkout a commit

[kitswas]

Closes #10068

Description

• All existing checkout infrastructure assumed that we will be checking out branches only. I modified it to allow both branches and commits. I created the helper functions for commit checkout following those for their branch counterparts.

Screen recording

https://user-images.githubusercontent.com/90329875/218293541-35ce9186-3d7b-41e7-b445-fd4e1997f0b5.mp4

Release notes

Notes: Added the much desired ability to checkout a single commit from the History tab. (#10068)

[dpizzo-at-aops]

I'm literally just an innocent bystander who wants to see this feature as well and decided to browse the PR for fun... sorry if I got your hopes up about someone who actually could merge this reviewing it 😅

[kitswas]

Thanks for the review.

if I got your hopes up

You did but don't worry about it. Any bit of activity on this PR should help.

[Gakk]

I am also just an innocent bystander who wants to see this feature implemented, and after watching your video I have one remark related to the user interface.

The feature is implemented so it is easy to checkout a commit — maybe too easy?

Most other tools shows a warning when user performs an action that will create a detached head. Maybe it will be easier to accept this pull request if a warning and choice is given to the user?

Example from Atlassian Sourcetree:

[kitswas]

You have raised a good point. Let's wait and see what the maintainers have to say.

Maybe it will be easier to accept

We won't know anything till the maintainers comment on what could be acceptable.

[Andrej730]

Got some idea and just wanted to put it out there.

Now when you checkout commit you see "Current branch" field changing to "Detached HEAD on hash"

Maybe it's just me but noticing this makes me expect that I'd be able to also drag'n'drop commit to "Current branch" to check it out.

[tidy-dev]

@kitswas Thank you for wanting to contribute and sorry for taking so long to respond. The team has just discussed this PR and we would like to see a warning like suggested in https://github.com/desktop/desktop/pull/16120#issuecomment-1508513704. If this is still something you are interested and able to work on, let us know.

[kitswas]

~~_{I should be studying for my exams now. But, sometimes, programming wins over reason. I have no idea why I am doing this past midnight.}~~

Here you go.

[kitswas]

@tidy-dev Ready for review.

[tidy-dev]

Still looking through this PR a bit, some stuff overlaps heavily with checkout branch process and going to confer with team on where it makes sense to share logic.

[kitswas]

We usually pin new features behind a feature flag. See feature-flag.ts for examples. Any entry point to the new feature should be flagged.

With Development Features enabled / GITHUB_DESKTOP_PREVIEW_FEATURES environment variable set:

[kitswas]

Remove all the formatting changes so reviewing this PR can be focused on code changes.

1. How may I do that?
2. I execute yarn lint:fix before commiting to this project. Why does the linter add spaces randomly (for indentation) and then remove them when run again (without any code changes)?
3. GitHub Desktop has a handy 'Hide whitespace changes' option that might help the reviewers.

[tidy-dev]

How may I do that?

I use Visual Studio Code and have prettier extenstion installed, and that is what automatically applies the formatting for me. My assumption is you have a different formatter managing your code? For now focus on the code changes, we can figure out the formatting later.

[tidy-dev]

@sergiou87 In regards to,

I did notice that when there are changes in the working directory, checking out a commit doesn't trigger the prompt suggesting to stash the changes and continue. However, I think that could be done in a different PR

We discussed this in a team sync a while ago and the reason we did not push for it in this PR is because if a user checks out a commit and then makes changes and attempts to check out a commit. A stash does not make sense because there is no branch to associate it with. Thus, implementing that needed more discussion around if we want to have a different behavior for when on branch versus on a detached head. Also as you say, if wanted, it can be done in a different PR.

[tidy-dev]

@kitswas Do you feel this is ready for another review?

[sergiou87]

@tidy-dev you're completely right, we discussed it and I was like " 💡 yeah! that makes total sense 🤯 " and then forgot about it 😂😂😂 I'm gonna give it a final review