Skip to content

git: Fix git message parsing #329

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.

Sign up for GitHub

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

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

git: Fix git message parsing #329

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/git.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,21 @@ pub fn read(options: ReadCommitMessageOptions) -> Vec<String> {
(None, None) => "HEAD".to_string(),
};

let opt_target = if let "HEAD" = range.as_str() {
"^FETCH_HEAD".to_string()
} else {
String::new()
};

// See https://git-scm.com/docs/git-log
let stdout = Command::new("git")
.arg("log")
.arg("--pretty=%B")
.arg("--pretty=%B%n|%n%nAuthor: %aN <%aE>%nDate: %ad%ncommit %H")
.arg("--no-merges")
.arg("--no-decorate")
.arg("--reverse")
.arg(range)
.arg(opt_target)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also quite handy - in case no range is specified, this will ensure we compare only current branch - IMO quite major use-case for regular every day development.
You don't care for commit linting on master, since you cannot usually just rebase it as you want. -> also if all commits that are merged to master are already valid by commitlint, no need to run it there at all...

.arg("--") // Explicitly specify the end of options as described https://git-scm.com/docs/git-log#Documentation/git-log.txt---ltpathgt82308203
.arg(options.path)
.output()
Expand All @@ -55,8 +62,10 @@ fn extract_commit_messages(input: &str) -> Vec<String> {

for commit in commits {
let message_lines: Vec<&str> = commit.trim().lines().collect();
let message = message_lines.join("\n");
messages.push(message);
if !message_lines.is_empty() {
let message = message_lines.join("\n");
messages.push(message);
}
}

messages
Expand Down Expand Up @@ -89,7 +98,7 @@ pub fn parse_commit_message(

for line in lines_iter {
if line.trim().is_empty() {
if in_body {
if in_body || line.trim() == "|" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case there is no body, this is necessary as we will skip the body and the "|" will help us switch to footers.

Copy link
Owner

@KeisukeYamashita KeisukeYamashita Aug 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you!

I would like to have a unit test for no body test case 🙏

in_body = false;
in_footer = true;
}
Expand Down