Re: [PATCH v3 2/4] branch: fix die_if_checked_out() when ignore_current_worktree
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Feb 7, 2023, 10:50 UTC
- Message-ID
- <1da499d6-3de3-ba34-1ca0-6f39e528b80b@dunelm.org.uk>
- In-Reply-To
- <230206.861qn27mt9.gmgdl@evledraar.gmail.com>
On 06/02/2023 16:56, Ævar Arnfjörð Bjarmason wrote:
Show 13 quoted lines
>
> On Sun, Feb 05 2023, Rubén Justo wrote:
>
>> - wt = find_shared_symref(worktrees, "HEAD", branch);
>> - if (wt && (!ignore_current_worktree || !wt->is_current)) {
>> - skip_prefix(branch, "refs/heads/", &branch);
>> - die(_("'%s' is already checked out at '%s'"), branch, wt->path);
>> + for (int i = 0; worktrees[i]; i++) {
>
> I see that there are existing "int i" for counting worktrees in
> worktree.c, FWIW for new code I wouldn't mind if it's "size_t i"
> instead, to make it future proof (and to eventually get rid of cast
> warnings as we move more things from "int" to "size_t").This seems to be different from the usual worries about int/size_t comparisons/truncation. worktrees is NULL terminated so there is no signed/unsigned comparison here as we're not comparing it to anything. The only concern would be that there are more than INT_MAX which seems unlikely.
Best Wishes
Phillip
Show 12 quoted lines
>> @@ -435,10 +435,9 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,
>> const char *target)
>> {
>>
>> - for (int i = 0; worktrees[i]; i++) {
>> + for (int i = 0; worktrees[i]; i++)
>> if (is_shared_symref(worktrees[i], symref, target))
>> return worktrees[i];
>> - }
>
> You added this function in the last commit, let's just skip adding the
> braces to begin with, rather than this style-fix after the fact.