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

git lfs status --json only includes lfs files #2374

Merged
merged 1 commit into from Jul 5, 2017
Merged

git lfs status --json only includes lfs files #2374

merged 1 commit into from Jul 5, 2017

Conversation

asottile
Copy link
Contributor

@asottile asottile commented Jul 2, 2017

More from #2289

Copy link
Contributor

@ttaylorr ttaylorr left a comment

Choose a reason for hiding this comment

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

Thanks so much for this, @asottile. I double checked that this behavior is consistent with LFS before 2.1.0, so I think the idea is good here.

I left a few minor remarks below, after which we should be able to merge without a problem:

@@ -244,9 +244,25 @@ func jsonStagedPointers(ref string) {
ExitWithError(err)
}

scanner, err := lfs.NewPointerScanner()
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this is being created in order to pass it to the blobInfoFrom call on L257 below. This is fine, though I'd rather pass the *lfs.PointerScanner we have in the statusCommand top-level function, like this:

diff --git a/commands/command_status.go b/commands/command_status.go
index e74e26b5..bfda7f8e 100644
--- a/commands/command_status.go
+++ b/commands/command_status.go
@@ -30,11 +30,18 @@ func statusCommand(cmd *cobra.Command, args []string) {
 		scanIndexAt = git.RefBeforeFirstCommit
 	}
 
+	scanner, err := lfs.NewPointerScanner()
+	if err != nil {
+		scanner.Close()
+
+		ExitWithError(err)
+	}
+
 	if porcelain {
 		porcelainStagedPointers(scanIndexAt)
 		return
 	} else if statusJson {
-		jsonStagedPointers(scanIndexAt)
+		jsonStagedPointers(scanner, scanIndexAt)
 		return
 	}
 
@@ -44,14 +51,6 @@ func statusCommand(cmd *cobra.Command, args []string) {
 	if err != nil {
 		ExitWithError(err)
 	}
-
-	scanner, err := lfs.NewPointerScanner()
-	if err != nil {
-		scanner.Close()
-
-		ExitWithError(err)
-	}
-
 	Print("\nGit LFS objects to be committed:\n")
 	for _, entry := range staged {
 		switch entry.Status {
@@ -238,7 +237,7 @@ type JSONStatus struct {
 	Files map[string]JSONStatusEntry `json:"files"`
 }
 
-func jsonStagedPointers(ref string) {
+func jsonStagedPointers(s *lfs.PointerScanner, ref string) {
 	staged, unstaged, err := scanIndex(ref)
 	if err != nil {
 		ExitWithError(err)
@@ -247,6 +246,9 @@ func jsonStagedPointers(ref string) {
 	status := JSONStatus{Files: make(map[string]JSONStatusEntry)}
 
 	for _, entry := range append(unstaged, staged...) {
+		_, from, err := blobInfoFrom(s, entry)
+		// ...
+
 		switch entry.Status {
 		case lfs.StatusRename, lfs.StatusCopy:
 			status.Files[entry.DstName] = JSONStatusEntry{


git commit -m "file1.dat -> file2.dat"

echo hi > test1.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding this test case! Would you mind either:

  • Adding a # comment above saying what this is testing, or...
  • Extracting this out to a separate test?

@asottile
Copy link
Contributor Author

asottile commented Jul 5, 2017

Thanks! Updated :D

@ttaylorr ttaylorr merged commit 35f6635 into git-lfs:master Jul 5, 2017
@ttaylorr
Copy link
Contributor

ttaylorr commented Jul 5, 2017

Thanks again for your contributions to LFS! I'll schedule this for v2.2.1.

@ttaylorr ttaylorr added this to the v2.2.1 milestone Jul 5, 2017
@asottile asottile deleted the lfs_status_only_lfs branch July 7, 2017 23:26
@asottile
Copy link
Contributor Author

asottile commented Jul 7, 2017

Awesome! Looking forward to 2.2.1 :D

ttaylorr added a commit that referenced this pull request Jul 10, 2017
Backport #2374 for v2.2.x: git lfs status --json only includes lfs files
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.

None yet

2 participants