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

cli: Color more change and commit ids in cli outputs #4475

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Veykril
Copy link
Collaborator

@Veykril Veykril commented Sep 15, 2024

Unlike #4444, this does not go through the relevant template. Should probably be doing that instead (though the template engine is pretty annoying to setup/use currently imo).

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

write!(formatter, " to ")?;
format_commit_shortest_prefix(formatter, repo, id_prefix_context, new_target)?;
}
writeln!(formatter)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps, this function (and print_git_import_stats()) can be extracted to templates similar to bookmark_list.

Each (String, BookmarkPushUpdate) could be encoded as commit_templater::RefName, but it's probably better to add RefDiff or RefChange template type. In print_git_import_stats(), we'll need to represent a diff of remote-ref to remote-ref.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes they ought to be all templated I noticed that after I was done with this current version (hence the draft).

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.

3 participants