This doesn't take into account the fact that usernames are case-insensitive. If we try to assign the same user twice, but use a non-canonical capitalization of the username, the output message won't be what we expect.

Cyborus marked this conversation as resolved

src/issues.rs

Show resolved Hide resolved

| | | | @ -886,0 +941,4 @@ | | | | | | let num_added = assigned_after - assigned_before; | | | | | | let num_duplicate = num_to_add - num_added; | | | | | | print!("assigned "); | | | | | | if num_added == 0 { |

LordMZTE commented 2026-04-16 17:16:35 +02:00

Collaborator

Copy link

This can be simplified by combining the zero and many if branches.

Cyborus commented 2026-04-16 17:26:06 +02:00

Author

Member

Copy link

oops, yes, that was a leftover from an earlier draft of that part where all users were already assigned was part of this if block too.

Cyborus marked this conversation as resolved

src/issues.rs

Show resolved Hide resolved

| | | | @ -886,0 +960,4 @@ | | | | | | } else if num_duplicate == 1 { | | | | | | println!(" (1 user was already assigned)") | | | | | | } else { | | | | | | println!(" ({num_duplicate} users was already assigned)") |

LordMZTE commented 2026-04-16 17:18:52 +02:00

Collaborator

Copy link

s/was/were

Cyborus marked this conversation as resolved

src/issues.rs

Show resolved Hide resolved

LordMZTE commented 2026-04-16 17:13:15 +02:00

Collaborator

Copy link

This only works when we capitalize the username correctly.

Cyborus commented 2026-04-16 17:29:47 +02:00

Author

Member

Copy link

Gonna check if Forgejo supports non-ascii usernames or not, which will determine if I should use .to_lowercase or if I can more optimally use .eq_ignore_ascii_case

Cyborus commented 2026-04-16 17:33:09 +02:00

Author

Member

Copy link

Indeed it is ASCII-only!

[image.png](https://codeberg.org/attachments/875f97bf-c628-4435-bcf0-551a9ba4c884 "Click to see "image.png" in a new tab")

15 KiB

Cyborus marked this conversation as resolved

src/issues.rs

Show resolved Hide resolved

| | | | @ -886,0 +984,4 @@ | | | | | | let assigned_before = assignees.len(); | | | | | | assignees.retain(|name| !users.contains(name)); | | | | | | let assigned_after = assignees.len(); | | | | | | let opt = forgejo_api::structs::EditIssueOption { |

LordMZTE commented 2026-04-16 17:14:43 +02:00

Collaborator

Copy link

More of a remark than a review comment: We should really add a Default::default implementation for this struct in forgejo-api and use ..Default::default() syntax here.

👍 1

Cyborus marked this conversation as resolved

src/issues.rs

Show resolved Hide resolved

| | | | @ -886,0 +1001,4 @@ | | | | | | let num_removed = assigned_before - assigned_after; | | | | | | let num_duplicate = num_to_remove - num_removed; | | | | | | print!("unassigned "); | | | | | | if num_removed == 0 { |

LordMZTE commented 2026-04-16 17:16:49 +02:00

Collaborator

Copy link

Ditto

Cyborus marked this conversation as resolved

src/issues.rs

Show resolved Hide resolved

| | | | @ -886,0 +1020,4 @@ | | | | | | } else if num_duplicate == 1 { | | | | | | println!(" (1 user was already not assigned)") | | | | | | } else { | | | | | | println!(" ({num_duplicate} users was already not assigned)") |

LordMZTE commented 2026-04-16 17:18:25 +02:00

Collaborator

Copy link

s/was/were

Cyborus marked this conversation as resolved

Cyborus force-pushed cyborus/assigning-issues from f9008a7d03

All checks were successful

ci/woodpecker/pr/check Pipeline was successful

Details

to f37bea70ff

All checks were successful

ci/woodpecker/pr/check Pipeline was successful

Details

2026-04-16 17:34:41 +02:00 Compare

Cyborus requested review from LordMZTE 2026-04-16 17:45:49 +02:00

LordMZTE approved these changes 2026-04-16 17:54:45 +02:00 Dismissed

LordMZTE left a comment

Copy link

Apart from the above review comment still left open, lgtm!

Cyborus commented 2026-04-16 17:57:15 +02:00

Author

Member

Copy link

Oh shoot how did I miss that one

Cyborus dismissed LordMZTE's review 2026-04-16 18:04:35 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Cyborus force-pushed cyborus/assigning-issues from f37bea70ff

All checks were successful

ci/woodpecker/pr/check Pipeline was successful

Details

to c6b6d7b14c 2026-04-16 18:04:42 +02:00 Compare

Cyborus force-pushed cyborus/assigning-issues from c6b6d7b14c to 8b82c17b43

All checks were successful

ci/woodpecker/pr/check Pipeline was successful

Details

2026-04-16 18:09:34 +02:00 Compare

Cyborus merged commit 3de18597cd into main 2026-04-16 18:18:49 +02:00

Cyborus referenced this pull request from a commit 2026-04-16 18:18:51 +02:00 Merge pull request 'feat: assigning issues' (#414) from cyborus/assigning-issues into main

Cyborus added this to the v0.5.0 milestone 2026-04-16 20:24:42 +02:00

stalecontext referenced this pull request from stalecontext/forgejo-cli-plus 2026-07-02 21:51:40 +02:00 Upstream sync 2026-07-02 #52

Reviewers

No reviewers

Labels

Clear labels[ Kind/Breaking Breaking change that won't be backward compatible

](#)[ Kind/Bug Something is not working

](#)[ Kind/Design Discussion about UI/UX design

](#)[ Kind/Documentation Documentation changes

](#)[ Kind/Enhancement Improve existing functionality

](#)[ Kind/Feature New functionality

](#)[ Kind/Security This is security issue

](#)[ Kind/Testing Issue or pull request related to testing

](#)[ Kind/Upstream This is an issue with upstream software (Forgejo) that is probably not our fault

](#)

[ Priority

Critical The priority is critical

](#)[ Priority

High The priority is high

](#)[ Priority

Low The priority is low

](#)[ Priority

Medium The priority is medium

](#)

[ Reviewed

Confirmed Issue has been confirmed

](#)[ Reviewed

Duplicate This issue or pull request already exists

](#)[ Reviewed

Invalid Invalid issue

](#)[ Reviewed

Won't Fix This issue won't be fixed

](#)

[ Status

Abandoned Somebody has started to work on this but abandoned work

](#)[ Status

Blocked Something is blocking this issue or pull request

](#)[ Status

Need More Info Feedback is required to reproduce issue or to continue work

](#)

[ Suspicious

](#)

No labels Kind/Breaking Kind/Bug Kind/Design Kind/Documentation Kind/Enhancement Kind/Feature Kind/Security Kind/Testing Kind/Upstream [ Priority

Critical ](/forgejo-contrib/forgejo-cli/pulls?labels=173012) [ Priority

High ](/forgejo-contrib/forgejo-cli/pulls?labels=173013) [ Priority

Low ](/forgejo-contrib/forgejo-cli/pulls?labels=173015) [ Priority

Medium ](/forgejo-contrib/forgejo-cli/pulls?labels=173014) [ Reviewed

Confirmed ](/forgejo-contrib/forgejo-cli/pulls?labels=173007) [ Reviewed

Duplicate ](/forgejo-contrib/forgejo-cli/pulls?labels=173005) [ Reviewed

Invalid ](/forgejo-contrib/forgejo-cli/pulls?labels=173006) [ Reviewed

Won't Fix ](/forgejo-contrib/forgejo-cli/pulls?labels=173008) [ Status

Abandoned ](/forgejo-contrib/forgejo-cli/pulls?labels=173011) [ Status

Blocked ](/forgejo-contrib/forgejo-cli/pulls?labels=173010) [ Status

Need More Info ](/forgejo-contrib/forgejo-cli/pulls?labels=173009) Suspicious

Milestone

Clear milestone

No items

No milestone v0.5.0

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo-contrib/forgejo-cli!414

WritePreview

Loading…

Add table

CancelOK

Add a link

Url Description Hint: With a URL in your clipboard, you can paste directly into the editor to create a link.

CancelOK

CancelSave

Reference in a new issue

Repository

forgejo-contrib/forgejo-cli

Title

Body

Create issue

No description provided.

Delete branch "cyborus/assigning-issues"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?

No Yes