fix: address all four open CLI issues (#323, #326, #328, #329) - #330
fix: address all four open CLI issues (#323, #326, #328, #329)#330devin-ai-integration[bot] wants to merge 1 commit into
Conversation
- omit dangling em-dash in format_fix_pr when the fix PR URL is empty - strip jj's '(push: ...)' annotation when parsing the origin remote - clamp requested page to the filtered total in bugs/scans list - split HTTPS credentials on the last '@' so passwords may contain '@' Co-Authored-By: Sachin Iyer <siyer@detail.dev>
Original prompt from Sachin
|
Current Aviator status
This PR is not ready to merge (currently in state pending): this PR has not been approved. Pending Status Checks
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Runtime verification (all four fixes)Built this branch and a 1+2.
|
Summary
All four open issues in this repo were small, well-localized correctness bugs, each fixed with a unit test:
bugs showdisplays dangling em-dash when Fix PR URL is empty #323format_fix_prunconditionally appended— {url}, so an emptyFixPr.urlrendered#42 (merged) —. Now branches onurl.is_empty(), matchingformat_linked_issue/Bug::to_card.parse_jj_remote_listreturned everything afterorigin, including jj's(push: <url>)annotation, which flowed through as a garbage repo id (owner/repo.git (push: https:) that passed validation. Now splits on" (push:"and keeps the fetch URL.bugs list/scans listpaths paginated with the raw requested page, producingPage: 5 of 1. Newoutput::clamp_page(page, total, limit) -> page.clamp(1, total_pages(total, limit))is applied beforepaginate_items/output_liston both filtered paths, so the page number and the items shown stay consistent.strip_http_credentialsusedsplit_once('@'); the credentials/host separator is the last@, sohttps://user:p@ss@github.com/owner/repo.gityielded hostss@github.comand inference returnedNone. Switched torsplit_once('@'), matchingstrip_ssh_portin the same file.Note: the local toolchain had to be updated to current stable (
chacha20 0.10.1requires edition 2024) to build;cargo fmt --check,cargo clippy -- -D warnings,cargo test, andcargo xtask checkall pass.cargo clippy --all-targetson current stable flags two preexisting lints untouched by this PR (src/commands/completions.rs:59redundant closure, trailing commas intests/integration.rs); CI only runscargo clippywithout--all-targets, so they don't gate this PR.Closes #323, closes #326, closes #328, closes #329.
Link to Devin session: https://app.devin.ai/sessions/fb903d8badf449c69bcb0e3b20a52a30
Requested by: @sachiniyer
Summary by cubic
Fixes four CLI correctness bugs in PR formatting, remote URL parsing, pagination, and HTTPS credential handling to improve output accuracy and repo inference. Adds unit tests for each fix.
FixPr.urlis empty informat_fix_pr([Detail Bug] CLI:bugs showdisplays dangling em-dash when Fix PR URL is empty #323).(push: <url>)annotation inparse_jj_remote_listto keep the fetch URL ([Detail Bug] CLI: Inferring GitHub repo from jj remote list includes '(push: ...)' annotation, producing malformed repo IDs #326).bugs listandscans listto keep pages and items consistent ([Detail Bug] CLI: Filtered scans/bugs list can show impossible pagination (page > total pages) #328).@so passwords with@parse correctly ([Detail Bug] VCS: Repo auto-inference fails for HTTPS remotes when password/token contains '@' #329).Written for commit 936f2b0. Summary will update on new commits.