[engine] getCommitRaws 함수 개선

yoouyeon commented 3 weeks ago

Related issue

closes #675
690
691

Result

getCommitRaws의 복잡한 파싱 로직을 단순화하고, 실행 속도를 (조금) 단축시켰습니다.

String.split()을 통해 간단하게 데이터를 분리할 수 있도록 format 변경
전체 git log 위에서 동작하던 파싱 로직을 각 commit 단위로 동작하도록 변경해서 commit 단위로 다시 데이터를 합치는 반복문 제거

AS-IS

git log format

commit <commit-hash> <parent-hash> <ref>
Author:     <author-name> <<author-email>>
AuthorDate: <author-date>
Commit:     <committer-name> <<committer-email>>
CommitDate: <committer-date>

    <commit-message-title>

    <commit-message-body>

<diffstat (differenceStatistic)>

`getCommitRaws` 실행 시간	githru-vscode-ext 레포지토리에서	argo-cd 레포지토리에서

TO-BE

git log format

[COMMIT_SEPARATOR]<commit-hash>[GIT_LOG_SEPARATOR]<parent-hash>[GIT_LOG_SEPARATOR]<ref>[GIT_LOG_SEPARATOR]<author-name>[GIT_LOG_SEPARATOR]<author-email>[GIT_LOG_SEPARATOR]<author-date>[GIT_LOG_SEPARATOR]<committer-name>[GIT_LOG_SEPARATOR]<committer-email>[GIT_LOG_SEPARATOR]<committer-date>[GIT_LOG_SEPARATOR]<commit-message>[GIT_LOG_SEPARATOR]
<diffstat (differenceStatistic)>

`getCommitRaws` 실행 시간	githru-vscode-ext 레포지토리에서	argo-cd 레포지토리에서

테스트 결과

Work list

getGitLog에서 받아오는 git log의 format 변경
변경한 format에 맞게 getCommitRaws의 파싱 로직 수정 (#680)
getCommitRaws 테스트 코드 추가 (#708, #735)

Discussion

지금은 Separator를 난수 문자열로 사용하고 있기 때문에

git log의 가독성이 나쁨
(가능성은 낮지만) 데이터에 난수가 포함되는 경우 잘못 파싱될 수 있음

의 문제가 있어 Separator를 다른 것으로 바꿀 필요가 있습니다.

대안으로 new line (\n)을 사용하는 방법이 있고, 지금까지는 가장 좋은 대안이라는 생각이 드는데요. 🥲 테스트해본 결과 (https://github.com/githru/githru-vscode-ext/pull/708#issuecomment-2334205508) \n을 사용하려면 git log에서 diffstat을 따로 불러오는 등의 추가적인 작업이 많이 필요할 듯 하여,, Separator 변경은 후속 이슈로 남겨두고 다른 대안이나 diffstat을 따로 불러오는 것에 대한 충분한 조사와 논의 후에 적용하는 것이 어떨까 싶습니다.. 🥹🥹

BeA-Pro commented 3 weeks ago

고생하셨습니다👍👍👍 테스트 코드 작성 시에 많이 여쭤봐야겠네요!

ytaek commented 3 weeks ago

@yoouyeon 님, https://github.com/githru/githru-vscode-ext/pull/708#issuecomment-2334205508 에서 언급하신 부분은,

각 Commit 내의 데이터를 구분하기 위한 GIT_LOG_SEPARATOR는 말씀해주신대로 commit message를 가장 아래쪽으로 옮긴다면 \n\n으로 할 수 있을 것 같지만 format을 지정하는 --pretty 옵션으로는 --numstat으로 출력하는 diffstat을 처리할 수 없는 것으로 알고 있습니다...

기존 코드들을 보면 pretty=fuller 옵션을 주고, https://github.com/githru/githru-vscode-ext/blob/68d4aaaeb3074f3ca16a1c02eca96678f770ed92/packages/vscode/src/utils/git.util.ts#L164

End of message인 \n 이 나올 때 까지 message 로 인식합니다. (fuller가 알아서 body는 indentation 해줌) https://github.com/githru/githru-vscode-ext/blob/68d4aaaeb3074f3ca16a1c02eca96678f770ed92/packages/analysis-engine/src/parser.ts#L68-L80

저희도 옵션을 잘 활용하면 body 에다가 앞에 space를 주면서 다른 내용들(numstat)과 구분이 가게 할 수 있습니다.

git --no-pager log --all --parents --date-order --pretty='format:%H%n%P%n%D%n%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%w(120,8)%s%w(120,8)%b%n' --numstat -1

위처럼 %w를 사용하면 가능하구요 (한참 찾았네요;;;) 이렇게 하면 별도의 separator의 정의 없이, git log에서 활용하는 \n을 그대로 이용해서 작업가능합니다 😺

yoouyeon commented 3 weeks ago

@ytaek Indentation을 넣는 방법은 생각을 못했었네요..!! 😲😲 직접 코드에 적용해봐야 정확히 알 수 있겠지만, 이 방법으로 GIT_LOG_SEPARATOR는 \n으로 대체할 수 있을 것 같습니다 (감사합니다..! 😵‍💫🥹🙇‍♀️) 하지만 여전히 각 커밋들을 구분하는 COMMIT_SEPARATOR를 대체하는 방법은 좀 고민이 되네요... 😵‍💫

이 PR은 머지하고, separator에 대한 작업은 #759 에서 이어서 하도록 하겠습니다!! 🔥

githru / githru-vscode-ext