[Approved] RFR: 92: GitRepository::currentBranch fails when on a detached HEAD

[Erik Helin]

On Fri, 29 Nov 2019 09:14:12 GMT, Robin Westberg <rwestberg at openjdk.org> wrote:

> On Thu, 28 Nov 2019 15:10:50 GMT, Erik Helin <ehelin at openjdk.org> wrote:
> 
>> Hi all,
>> 
>> please review this pull request that makes `git webrev` work when the repository is in a "detached HEAD" state (i.e. there is no active branch). The main part of the change is making `ReadOnlyRepository::currentBranch` return `Optional<Branch>` instead of `Branch`.
>> 
>> Thanks,
>> Erik
>> 
>> ## Testing
>> - [x] `make test` on Linux x64
>> - [x] Manually running `git webrev` on a repository with a detached HEAD
>> 
>> ----------------
>> 
>> Commits:
>>  - 3dc4fea9: 92: GitRepository::currentBranch fails when on a detached HEAD
>> 
>> Changes: https://git.openjdk.java.net/skara/pull/278/files
>>  Webrev: https://webrevs.openjdk.java.net/skara/278/webrev.00
>>   Issue: https://bugs.openjdk.java.net/browse/SKARA-92
>>   Stats: 40 lines in 8 files changed: 16 ins; 2 del; 22 mod
>>   Patch: https://git.openjdk.java.net/skara/pull/278.diff
>>   Fetch: git fetch https://git.openjdk.java.net/skara pull/278/head:pull/278
> 
> Looks good! Perhaps the error message could be made a bit more helpful, but not sure what it should say. :)
> 
> cli/src/main/java/org/openjdk/skara/cli/GitPr.java line 523:
> 
>> 522:             var currentBranch = repo.currentBranch().orElseGet(() -> {
>> 523:                     System.err.println("error: the repository is in a detached HEAD state");
>> 524:                     System.exit(1);
> 
> Suggestion:
> 
>                     System.err.println("error: the repository is in a detached HEAD state. check out a branch to continue.. or something");
> 
> ----------------
> 
> Approved by rwestberg (Reviewer).

I thought about this as well, but if you see the "detached HEAD" warning when doing e.g. `git checkout HEAD~` there is just no way we are gonna be able to write all that ��

So I figured that the next best thing was to write something concise that is very easy to search for. If the user does not know what "detached HEAD state" means then he/she will immediately search for it and will most likely stumble upon [this explanation](https://www.git-tower.com/learn/git/faq/detached-head-when-checkout-commit) which is the first hit on both DuckDuckGo and Google for me.

I think that is the best alternative here, what do you say?

PR: https://git.openjdk.java.net/skara/pull/278