Skip to content

Add edit subcommand#36

Closed
tekknolagi wants to merge 2 commits intodanobi:masterfrom
tekknolagi:mb-add-edit-command
Closed

Add edit subcommand#36
tekknolagi wants to merge 2 commits intodanobi:masterfrom
tekknolagi:mb-add-edit-command

Conversation

@tekknolagi
Copy link
Contributor

@tekknolagi tekknolagi commented Dec 11, 2023

Works like get except it opens up the specified PR in $EDITOR.

Fixes #2.

Works like `get` except it opens up the specified PR in `$EDITOR`.
Copy link
Owner

@danobi danobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the pr! minor ux comments.

/// Pull request to review (eg. `danobi/prr/24`)
pr: String,
},
/// Get a pull request and open it in $EDITOR
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than a new subcommand to get + open, how about making prr-edit only open a review file if it exists (so no implicit prr-get). And also add a --open flag to prr-get?

Comment on lines +108 to +112
let (owner, repo, pr_num) = prr.parse_pr_str(&pr)?;
let review = prr.get_pr(&owner, &repo, pr_num, force).await?;
let editor = env::var("EDITOR")?;
let path = review.path();
std::process::Command::new(editor)
Copy link
Owner

@danobi danobi Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want to move this code into prr.rs if we wnat to share open functionality between prr-edit and prr get --open

Co-authored-by: Daniel Xu <accounts@dxuuu.xyz>
@tekknolagi
Copy link
Contributor Author

I can probably get to the rest later this week, or feel free to edit & submit.

@Shugyousha
Copy link
Contributor

std::process::Command::new(editor)

Couldn't we suggest adding a shell alias doing something like $EDITOR $(prr get your/pr-to-review/42) instead of adding this functionality into the application code? To me this would be a more elegant way of handling this use case

@danobi
Copy link
Owner

danobi commented Dec 12, 2023

@Shugyousha On principle I agree, but sometimes out-of-the-box functionality is good for ux. Plus, mail clients like mutt/neomutt will do similar things for replying to mail. So I don't mind adding this. Fwiw I will probably use prr get --open ... a lot more than a bare prr-edit command

@danobi
Copy link
Owner

danobi commented Dec 22, 2023

Some of the code in this PR went into 149a8fe

@danobi
Copy link
Owner

danobi commented Dec 25, 2023

Finished in e7f4e43

@danobi danobi closed this Dec 25, 2023
@tekknolagi
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: use EDITOR or default editor used by git to review

3 participants