feat(view, vscode): implement branchSelector onChange handler #486

ss-won · 2023-09-18T14:42:25Z

Related issue

[view] 기준 Branch 선택 UI #276

Result

초기 base branch는 현재 local repository의 HEAD 위치 (git branch --show-current)
- view 영역에서는 current branch를 따로 호출하지 않고 git branchList를 호출하면서 현재 head가 가리키고 있는 branch 전달 받음
branch selector를 통해 base branch 변경

Work list

vscode git util 함수 변경
- getCurrentBranch 함수 생성
  - git branch --show-current결과 값 return
- getBranches 함수 변경
  - 단순 branchList만 return하던 구조에서 -> 현재 HEAD가 위치한 branch도 함께 반환
BranchSelector onChange handler 구현
기타 리뷰 반영

Discussion

BranchSelector에서 현재 작업하고 있는 브랜치(HEAD)에 대한 특정 표시의 필요성 논의?
vscode 타입 정리 -> vscode도 진행하고 있는지는 모르겠지만, 오프라인 회의나 리뷰 후 정리 되면 아래 타입 추가된 타입 내용 파일로 따로 빼겠습니다 :).

type GithruFetcher<D = unknown, P extends unknown[] = []> = (...params: P) => Promise<D>;
type GithruFetcherMap = {
  fetchClusterNodes: GithruFetcher<ClusterNode[], [string]>;
  fetchBranches: GithruFetcher<{ branchList: string[]; head: string | null }>;
  fetchCurrentBranch: GithruFetcher<string>;
};

Note

추가적인 .git 디렉토리 관련 파일의 추가/변경에 대한 추적 필요성? https://vshaxe.github.io/vscode-extern/vscode/FileSystemWatcher.html

ytaek

드디어 이게 되는군요 :)
전체적으로 코드가 깔끔해져서 좋습니다.
추가로 comment달았는데, 추후에 반영 혹은 논의 부탁드리겠습니다~.

ytaek · 2023-09-19T06:03:14Z

packages/view/src/components/BranchSelector/BranchSelector.tsx

+    const ideAdapter = container.resolve<IDEPort>("IDEAdapter");
+    setLoading(true);
+    ideAdapter.sendFetchAnalyzedDataMessage(e.target.value);


지난번 refactoring 수행하면서 services/index.ts에 필요 api들을 모아두었습니다.
(바로 ideAdapter를 안 쓰고 한번 더 wrapping 했어요)
참고해주시고, 이번 or 다음 PR에서 수정 부탁드립니다~

오 넵넵 BranchSelector, RefreshButton에 적용해두겠습니다~

ytaek · 2023-09-19T06:05:15Z

packages/view/src/hooks/useGlobalData.ts

+  baseBranch: string;
+  setBaseBranch: Dispatch<SetStateAction<string>>;


base라고 해야할지, selected라고 하는게 맞을지, current라고 해야 할지 좀 헷갈리긴 하네요 ㅎㅎ
한번 고민해보면 좋겠습니다.

저도 3가지 중에서 고민을 했었다가 base branch를 추후 vscode에서 fetching 해오는 작업이 있을까 해서, baseBranch로 두었는데view에서는 selected로 두는게 직관적인것 같네요...!

이렇게 용어를 정리하면 될 것 같네요...🤔
view: selected branch (BranchSelector Component에서 선택한 값, 초기 값은 git HEAD의 위치)
↕️
vscode: selected(view에서 전달받은 값) branch or current branch (git branch --show-current)
↕️
engine: base branch (stem target base branch)

오. 완전 좋습니다!

ytaek · 2023-09-19T06:09:51Z

packages/vscode/src/extension.ts

+      const fetchBranches = async () => await getBranches(gitPath, currentWorkspacePath);
+      const fetchCurrentBranch = async () => await getCurrentBranchName(gitPath, currentWorkspacePath);


refactoring으로 깔끔해졌네요!! 👍👍👍👍👍

ytaek · 2023-09-19T06:13:19Z

packages/vscode/src/utils/git.util.ts

+export function getDefaultBranchName(branchList: string[]): string {
+  const branchSet = new Set(branchList);
+  return branchSet.has("main") ? "main" : branchSet.has("master") ? "master" : branchList?.[0];
+}


이 method는 언제 쓰이나요?

예전에 base branch 불러올때 해당 함수를 썼었는데 이제 git branch --show-current를 대상으로 하는 방식(#477 (comment)) 으로 바꾸면서 현재는 사용하지 않고 있긴 합니다. 삭제하도록 하겠습니다~

detached branch 상황에 대응하여 사용하는걸로 바꿨습니다. 기록용으로 코멘트 남깁니다 :)

packages/vscode/src/webview-loader.ts

ytaek · 2023-09-19T06:18:54Z

packages/vscode/src/webview-loader.ts

@@ -107,3 +108,10 @@ export default class WebviewLoader implements vscode.Disposable {
    return returnString;
  }
 }
+
+type GithruFetcher<D = unknown, P extends unknown[] = []> = (...params: P) => Promise<D>;
+type GithruFetcherMap = {


(naming) Map 보다는 명시적인게 더 좋을 것 같긴 합니다.
Githru는 빼도 괜찮을 것 같구요.
(react의 prop 같은 녀석이라고 생각해도 될 것 같고...)

혹은 extension.ts에서 fetcher들의 dependency를 없애고 아예
webview-loader쪽으로 가져와도 괜찮을 것 같습니다.

그냥 dependecy를 없애는게 장기적으로 좋을 것 같네요! 말씀하신 부분은 다음 PR로 끊어서 진행하겠습니다

ytaek

고생하셨습니다!!

KyuTae98

LGTM!!!

ss-won · 2023-09-19T12:58:46Z

closed: #276

feat: implement branchSelector onChange handler

1cb3103

ss-won requested review from a team as code owners September 18, 2023 14:42

ss-won self-assigned this Sep 18, 2023

ss-won added ✨ view 💻 vscode new feature labels Sep 18, 2023

ss-won changed the title ~~feat: implement branchSelector onChange handler~~ feat(view, vscode): implement branchSelector onChange handler Sep 18, 2023

ytaek approved these changes Sep 19, 2023

View reviewed changes

fix: apply view service module, add coverage about detached head

a80e141

ss-won mentioned this pull request Sep 19, 2023

[vscode] refactoring type naming, extension fetcher 함수 dependencies 제거 #488

Open

ytaek approved these changes Sep 19, 2023

View reviewed changes

KyuTae98 approved these changes Sep 19, 2023

View reviewed changes

ss-won merged commit ae3e28a into githru:main Sep 19, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(view, vscode): implement branchSelector onChange handler #486

feat(view, vscode): implement branchSelector onChange handler #486

ss-won commented Sep 18, 2023

ytaek left a comment

ytaek Sep 19, 2023

ss-won Sep 19, 2023 •

edited

Loading

ytaek Sep 19, 2023

ss-won Sep 19, 2023 •

edited

Loading

ytaek Sep 19, 2023

ytaek Sep 19, 2023

ytaek Sep 19, 2023

ss-won Sep 19, 2023 •

edited

Loading

ss-won Sep 19, 2023

ytaek Sep 19, 2023 •

edited

Loading

ss-won Sep 19, 2023

ytaek left a comment

KyuTae98 left a comment

ss-won commented Sep 19, 2023

		baseBranch: string;
		setBaseBranch: Dispatch<SetStateAction<string>>;

		const fetchBranches = async () => await getBranches(gitPath, currentWorkspacePath);
		const fetchCurrentBranch = async () => await getCurrentBranchName(gitPath, currentWorkspacePath);

feat(view, vscode): implement branchSelector onChange handler #486

feat(view, vscode): implement branchSelector onChange handler #486

Conversation

ss-won commented Sep 18, 2023

Related issue

Result

Work list

Discussion

ytaek left a comment

Choose a reason for hiding this comment

ytaek Sep 19, 2023

Choose a reason for hiding this comment

ss-won Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

ytaek Sep 19, 2023

Choose a reason for hiding this comment

ss-won Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

ytaek Sep 19, 2023

Choose a reason for hiding this comment

ytaek Sep 19, 2023

Choose a reason for hiding this comment

ytaek Sep 19, 2023

Choose a reason for hiding this comment

ss-won Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

ss-won Sep 19, 2023

Choose a reason for hiding this comment

ytaek Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

ss-won Sep 19, 2023

Choose a reason for hiding this comment

ytaek left a comment

Choose a reason for hiding this comment

KyuTae98 left a comment

Choose a reason for hiding this comment

ss-won commented Sep 19, 2023

ss-won Sep 19, 2023 •

edited

Loading

ss-won Sep 19, 2023 •

edited

Loading

ss-won Sep 19, 2023 •

edited

Loading

ytaek Sep 19, 2023 •

edited

Loading