Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(submit): Apply styles to branch names #685

Merged
merged 1 commit into from
Dec 11, 2022

Conversation

claytonrcarter
Copy link
Collaborator

This is a small patch to git submit to style the pushed/skipped branches with colors. My goal is to improve on the default output, which doesn't make it very easy to quickly identify the pushed/skipped branches w/o reading the output carefully.

Screen Shot 2022-11-28 at 9 42 16 PM

  • This was mostly lifted from git test and how it styles passing/skipped/failed tests.

Copy link
Owner

@arxanas arxanas left a comment

Choose a reason for hiding this comment

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

Thanks, this looks better! I think for the git test output, we use bold for both the green and the yellow. I think we should remain internally consistent in this case and either use bold for both sets of branches or neither. What do you think?

@claytonrcarter
Copy link
Collaborator Author

I can totally add the bold back in for consistency. I removed it b/c it felt a little heavy handed in this case, but I definitely don't feel strongly about it.

New issue, though: it looks like #650 removed cursive from git-branchless/Cargo.toml, but this PR adds a dep on cursive to git submit, which was not extracted to it's own crate. Which course of action would you prefer:

  1. Add cursive back to git-branchless/Cargo.toml just for this change
  2. Extract git submit as it's own crate a la Try to improve incremental build times for tests #650 (I think I can do that by following along to the changes in that PR)
  3. Abandon this change as not worth it ... no hard feelings; I promise. 😄

@claytonrcarter
Copy link
Collaborator Author

I've re-added the bold effect, and added temporary commit w/ a top-level dep on cursive just to get something working. I'll update this once I know what you'd prefer. Thanks!

@arxanas
Copy link
Owner

arxanas commented Dec 11, 2022

@claytonrcarter does it work to add a dependency on just cursive_core?

Extract git submit as it's own crate

This would be good to do at some point, but cursive_core isn't as big of a dependency (although I'm surprised that we don't already need it for output rendering anywhere else in the crate).

@claytonrcarter
Copy link
Collaborator Author

Aha! I hadn't done my homework. cursive_core is already a dependency, and yes it worked to just use that instead. Updated and pushed. Thank you!

@claytonrcarter claytonrcarter merged commit cad2e1a into arxanas:master Dec 11, 2022
@arxanas
Copy link
Owner

arxanas commented Dec 27, 2022

I'm liking the coloring so far!

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.

2 participants