forgejo-contrib/forgejo-cli
20
407
Fork
You've already forked forgejo-cli
51

fix(activity): parse issue content field as json #437

Merged
Cyborus merged 2 commits from cyborus/activity-issue-fix into main 2026年04月24日 14:49:08 +02:00 AGit
Member
Copy link

Changes Made

This was incorrectly being parsed by splitting on a | character, when it's actually a JSON array/tuple. I'm guessing this was changed in Forgejo at some point, as I don't know why I would have parsed it like this if it was always JSON.

Also pulls the repo name out into its own function and uses it in all places the repo name is accessed.

Code of Conduct

  • I agree to act in accordance with the CoC & AI Agreement.
  • This contribution was not generated by an LLM, even in part.
### Changes Made This was incorrectly being parsed by splitting on a `|` character, when it's actually a JSON array/tuple. I'm guessing this was changed in Forgejo at some point, as I don't know why I would have parsed it like this if it was always JSON. Also pulls the repo name out into its own function and uses it in all places the repo name is accessed. ### Code of Conduct - [x] I agree to act in accordance with the CoC & AI Agreement. - [x] This contribution was not generated by an LLM, even in part.
LordMZTE requested changes 2026年04月23日 12:06:50 +02:00
Dismissed
LordMZTE left a comment
Copy link

What seems a little odd to me is that we seem to be treating issue numbers as strings, but I guess that's needed because they're enclosed in quotes in the JSON we're parsing?

What seems a little odd to me is that we seem to be treating issue numbers as strings, but I guess that's needed because they're enclosed in quotes in the JSON we're parsing?
@ -765,1 +759,3 @@
Ok((full_name,issue_id))
.ok_or_eyre("repo does not have full name")
}
usestd::borrow::Cow;
Collaborator
Copy link

This use seems oddly placed. Don't we put those at the top of the file?

This `use` seems oddly placed. Don't we put those at the top of the file?
LordMZTE marked this conversation as resolved
@ -766,0 +759,4 @@
.ok_or_eyre("repo does not have full name")
}
usestd::borrow::Cow;
fn issue_content(content: &str)-> eyre::Result<(Cow<'_,str>,Cow<'_,str>)>{
Collaborator
Copy link

I think it'd be good to have a doc comment here explaining the meaning of the returned tuple, especially since I'm not sure what the second element is even after reading the code and description of this PR. We could also consider defining our own tuple struct with documented fields here.

I think it'd be good to have a doc comment here explaining the meaning of the returned tuple, especially since I'm not sure what the second element is even after reading the code and description of this PR. We could also consider defining our own tuple struct with documented fields here.
LordMZTE marked this conversation as resolved
Author
Member
Copy link

What seems a little odd to me is that we seem to be treating issue numbers as strings, but I guess that's needed because they're enclosed in quotes in the JSON we're parsing?

Yeah, they're strings in the jSON (and afaik serde can't parse numbers from that), and I don't bother .parse()ing it after either because the number only gets printed, so it'd be doing string -> number immediately followed by number -> string. Seems silly.

> What seems a little odd to me is that we seem to be treating issue numbers as strings, but I guess that's needed because they're enclosed in quotes in the JSON we're parsing? Yeah, they're strings in the jSON (and afaik `serde` can't parse numbers from that), and I don't bother `.parse()`ing it after either because the number only gets printed, so it'd be doing `string -> number` immediately followed by `number -> string`. Seems silly.
Cyborus force-pushed cyborus/activity-issue-fix from 75b8aae849
All checks were successful
ci/woodpecker/pr/check-typos Pipeline was successful
ci/woodpecker/pr/check Pipeline was successful
to 447091dfb4
All checks were successful
ci/woodpecker/pr/check-typos Pipeline was successful
ci/woodpecker/pr/check Pipeline was successful
2026年04月23日 15:14:02 +02:00 Compare
Cyborus added this to the v0.5.0 milestone 2026年05月08日 01:46:41 +02:00
Cyborus removed this from the v0.5.0 milestone 2026年05月08日 01:46:45 +02:00
Cyborus added this to the v0.6.0 milestone 2026年05月08日 01:48:16 +02:00
Sign in to join this conversation.
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
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
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!437
Reference in a new issue
forgejo-contrib/forgejo-cli
No description provided.
Delete branch "cyborus/activity-issue-fix"

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?