[vscode/engine] git log 파싱 로직 구조 개선 리팩토링 by nxnaxx · Pull Request #825 · githru/githru-vscode-ext

nxnaxx · 2025-08-10T01:45:06Z

Related issue

#821

Result

Git 로그 파싱 관련 아키텍처를 개선하는 리팩토링 작업을 진행했습니다.

Work list

1. parser 함수 로직 분리

[기존 문제점]

하나의 함수(getCommitRaws) 내에서 여러 책임을 담당(e.g. 데이터 파싱, 브랜치/태그 파싱)
단위 테스트가 어려워 통합 테스트만 가능 -> 문제 발생 시 디버깅이 힘든 구조

[개선 사항]

함수 내 로직을 책임별로 분리하여 가독성과 유지보수성을 향상시켰습니다.
기존 통합 테스트와 겹치지 않는 선에서 함수 단위 테스트를 추가하였습니다.

2. Git log format 추출

[기존 문제점]

동일한 git log format 문자열이 두 파일에서 중복 정의

[개선 사항]

개별 분리하여 상수로 정의하였습니다.
따로 constants 폴더를 생성하지 않고 utils 디렉터리 내 추가하였습니다.
-> git 유틸리티 함수에서만 쓰이는 상수이므로 내부 상태를 유지하는 것이 더 낫다고 판단하였습니다.

3. Worker 관리 로직 캡슐화

[기존 문제점]

git 로그 처리와 워커 관련 로직이 fetchGitLogInParallel 함수 내부에 혼재되어 있음
독립적인 테스트 어려움

[개선 사항]

워커 관리 로직을 별도의 클래스로 캡슐화하여 재사용성과 확장성을 향상시켰습니다.
관련 단위 테스트를 추가하였습니다.

4. 에러 처리 표준화

[기존 문제점]

git.util.ts와 git.worker.ts 내 일관되지 않은 에러 처리 방식

[개선 사항]

formatGitError 에러 처리 함수를 생성하여 표준화하였습니다.
에러 핸들러 관련 테스트를 추가하였습니다.

Discussion

-

ytaek

멋진 refactoring입니다!!!

단, 같은 기능을 유지하는 refactoring이라고 할지라도 일단 절대적인 코드양이 좀 많아서,
리뷰하기가 무척 힘든건 사실입니다 ㅜ.ㅜ

번거로우시더라도 다음번엔 항목들에 대해 좀 쪼개서 올려주시면 좋을 것 같습니다!!!
(특히 error 추가 이런 부분은 별도로 올리셨으면 더 좋았을 것 같습니다!)

ytaek · 2025-08-11T12:40:42Z

+    id,
+    parents,
+    refs,
+    authorName,
+    authorEmail,
+    authorDate,
+    committerName,
+    committerEmail,
+    committerDate,
+    ...messageAndDiffStats


(잘 모름) 뭔가 잘 떠오르지는 않지만, 아랫 부분이랑 동일한 코드라서, duplication을 없앨 수도 있을 것 같습니다 👿

저도 고민되었던 부분이었는데 짚어주셨네요! 감사합니다.
현재 리팩토링된 코드는 destructuring을 사용하고 있어 구조상 중복을 피할 수 없는 것이 단점이라고 생각합니다.

다른 방식도 고려해봤었는데요,

인덱스로 접근하는 방식 -> 중복은 줄일 수 있지만 의미 없는 숫자로 가독성이 떨어짐

key 배열은 한 번만 지정하고 동적 객체 생성 -> 중복은 줄일 수 있지만 코드가 길어지고 약간 장황해짐. 런타임 시 추가 연산 필요

git log 포맷은 이미 표준적이고 변경될 가능성이 낮아 간단하고 직관적인 destructuring 방식을 사용하는 것이 적합하다고 생각했습니다. 타입스크립트 자동 타입 추론도 사용할 수 있고요.

물론 다른 의견이나 더 나은 방법이 있다면 충분히 고려할 의향이 있습니다! 저도 아직은 잘 떠오르지 않습니다😅

ytaek · 2025-08-11T12:41:10Z

+export function parseDiffStatLine(line: string, diffStats: DifferenceStatistic) {
+  const [insertions, deletions, path] = line.split("\t");
+  const numberedInsertions = insertions === "-" ? 0 : Number(insertions);
+  const numberedDeletions = deletions === "-" ? 0 : Number(deletions);
+
+  diffStats.totalInsertionCount += numberedInsertions;
+  diffStats.totalDeletionCount += numberedDeletions;
+  diffStats.fileDictionary[path] = {
+    insertionCount: numberedInsertions,
+    deletionCount: numberedDeletions,
+  };
+}


extract method 👍👍👍👍👍

ytaek · 2025-08-11T12:43:33Z

  });
 });
+
+describe("getGitLog error handling", () => {


우왕... TC 가 아름답습니다!!!!!!!!! 🌷 🌹 🌷 🌹 🌷 🌹 🌷 🌹

ytaek · 2025-08-11T12:48:41Z

@@ -1,59 +1,46 @@
 import * as cp from "child_process";


이전 legacy의 이슈긴 하지만,
utils안의 ts 파일들의 naming이 다른 부분들과 좀 다른 것 같긴하네요.

test case를 포함하는 .spec.ts 파일들 외에 앞의 내용은 gitConstans, gitParallel 등으로 맞춰야 되는게 아닌가? 싶긴 합니당!

저도 의견에 동의합니다. 카멜케이스로 통일해서 반영해 놓도록 하겠습니다!

SingTheCode

에러 클래스를 분리해서 관리한게 너무 좋았고, getCommitRaws 내부로직이 추상화되어 흐름을 한눈에 볼 수 있는게 좋았습니다ㅎㅎ 고생하셨습니다!

SingTheCode · 2025-08-16T02:51:37Z

+    if (idx === 0)
+      // message subject
+      messageSubject = line;
+    else if (line.startsWith(INDENTATION)) {
+      // message body (add newline if not first line)
+      messageBody += idx === 1 ? line.trim() : `\n${line.trim()}`;
+    } else if (line === "")
+      // pass empty line
+      continue;
+    else {
+      // diffStats
+      parseDiffStatLine(line, diffStats);
+    }


코드만으로 설명이 충분해서 주석이 없어도 괜찮을 것 같습니다ㅎㅎ

이전 코드에 남아있던 주석이었는데, 저도 삭제하는 것이 좋다고 생각합니다. 반영해서 수정해놓겠습니다. 감사합니다!

반영해주셔서 감사합니다ㅎㅎ

SingTheCode · 2025-08-16T02:54:05Z

+  const commits = splitLogIntoCommits(log);
+  return commits.map((commit, idx) => {
+    const commitData = extractCommitData(commit);
+    const parsedRefs = parseRefsData(commitData.refs);
+    const parsedMessage = parseMessageAndDiffStats(commitData.messageAndDiffStats);
+    return createCommitRaw(idx, commitData, parsedRefs, parsedMessage);
+  });


복잡한 로직이 전부 추상화되어 getCommitRaws의 흐름을 따라가기가 훨씬 쉬워졌네요!! 고생하셨습니다ㅎㅎㅎ

SingTheCode · 2025-08-16T02:54:33Z

+  };
+}
+
+export default function getCommitRaws(log: string): CommitRaw[] {


개인적으로 궁금한 부분인데, 한 파일에서 export default와 export를 둘다 사용하기도 하나요??

넵 함께 사용해도 됩니다. getCommitRaws는 대표 함수여서 default로 지정하였고 나머지 헬퍼 함수들은 테스트 파일에서 임포트하려고 export 하였습니다. default만 파일 당 하나라면 문제 없는 것으로 알고 있습니다!

알려주셔서 감사합니다!!

SingTheCode · 2025-08-16T02:56:55Z

-      },
-      [new Array<string>(), new Array<string>()]
-    );
+export function parseRefsData(refs: string): ParsedRefs {


개인적으로는 어디서 파싱할 데이터를 가져오는지, 그래서 결과는 무엇인지가 함수명에서 명확하게 보이는 것을 좋아합니다ㅎㅎ

이 함수는 ExtractBranchesAndTags 가 더 명확하게 의미가 보이려나요?? 모든 parse 함수에 대한 의견 동일합니다ㅎㅎ 개인적인 의견이라 넘어가셔도 괜찮습니다!

제안주신 함수명이 훨씬 명확하고 좋네요! 함수명 지을 때마다 고민인데, 반환값이 드러나는 이름이 더 이해하기 쉬운 것 같아요. 하나 배워갑니다 감사합니다😄 수정하면서 다른 부분들도 고민해보겠습니다.

같이 고민해주셔서 감사합니다ㅎㅎㅎ

SingTheCode · 2025-08-16T02:59:14Z

+    super(message);
+    this.name = "WorkerThreadError";
+  }
+}


에러 클래스를 따로 만드신게 너무 좋은 것 같아요ㅎㅎ

nxnaxx · 2025-08-16T04:33:15Z

멋진 refactoring입니다!!!

단, 같은 기능을 유지하는 refactoring이라고 할지라도 일단 절대적인 코드양이 좀 많아서, 리뷰하기가 무척 힘든건 사실입니다 ㅜ.ㅜ

번거로우시더라도 다음번엔 항목들에 대해 좀 쪼개서 올려주시면 좋을 것 같습니다!!! (특히 error 추가 이런 부분은 별도로 올리셨으면 더 좋았을 것 같습니다!)

다음부터는 꼭 쪼개서 올리도록 하겠습니다!

ytaek

고생하셨습니다!! LGGGGTM!

SingTheCode

고생하셨습니다!!!ㅎㅎ

nxnaxx added 7 commits August 10, 2025 09:15

refactor(vscode): git log format 및 병렬 처리 설정값 분리

057f95b

refactor(vscode): git worker 관리 로직 모듈화

0d7ad28

test(vscode): git parallel worker 처리 로직 테스트 추가

d1cd31e

refactor(vscode): git 에러 처리 통합

66634da

test(vscode): git error handler 관련 테스트 추가

c870d90

refactor(engine): parser 로직 기능별로 분리

1531dc5

test(engine): parser 함수 단위 테스트 추가

3afa64a

nxnaxx requested a review from a team as a code owner August 10, 2025 01:45

ytaek previously approved these changes Aug 11, 2025

View reviewed changes

SingTheCode previously approved these changes Aug 16, 2025

View reviewed changes

nxnaxx added 2 commits August 16, 2025 23:44

chore(vscode): utils 하위 파일명 camelCase로 통일

962e52a

refactor(engine): parser.ts 내 헬퍼 함수명 변경 및 주석 제거

43bdf34

nxnaxx dismissed stale reviews from SingTheCode and ytaek via 43bdf34 August 16, 2025 15:21

ytaek approved these changes Aug 17, 2025

View reviewed changes

SingTheCode approved these changes Aug 17, 2025

View reviewed changes

nxnaxx merged commit 22b9703 into githru:main Aug 17, 2025
2 checks passed

nxnaxx self-assigned this Oct 2, 2025

Uh oh!

Conversation

nxnaxx commented Aug 10, 2025

Related issue

Result

Work list

1. parser 함수 로직 분리

2. Git log format 추출

3. Worker 관리 로직 캡슐화

4. 에러 처리 표준화

Discussion

Uh oh!

ytaek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SingTheCode left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nxnaxx commented Aug 16, 2025

Uh oh!

ytaek left a comment

Choose a reason for hiding this comment

Uh oh!

SingTheCode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants