Skip to content

Prevent staged rename from displaying unstaged changes#491

Closed
harish wants to merge 1 commit intojonas:masterfrom
harish:gh-472
Closed

Prevent staged rename from displaying unstaged changes#491
harish wants to merge 1 commit intojonas:masterfrom
harish:gh-472

Conversation

@harish
Copy link
Copy Markdown
Contributor

@harish harish commented Apr 6, 2016

#472 - Took a whack at this. Appears that git status --porcelain -z outputs "R dst\0src\0" for a rename.

@harish harish closed this Apr 6, 2016
@harish harish reopened this Apr 6, 2016
}

/* Skip source filename in rename */
if (buf.data[0] == 'R')
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Shouldn't renames increase the diff->staged count? This patch will report the index as clean.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will still get incremented at line 163 as it did before. The next iteration where it reads in the 2nd filename will skip. Originally I used a counter because I thought it'd hit two filenames. Would it be more clear to use a flag like 'skip_next' or 'last_was_rename'?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yes, I missed that. I ended up putting an explicit io_get call to read the source filename since ok = buf.size > 3 was breaking for short filenames.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe the || skip_count should've caught that case but I definitely like your version better-- much cleaner. Thanks :)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yes, it would have continued but ok would have become false due to the tricky assignment in the while condition causing the result of index_diff to be discarded

@jonas
Copy link
Copy Markdown
Owner

jonas commented Apr 13, 2016

I think it would make sense to add a test for this

@harish
Copy link
Copy Markdown
Contributor Author

harish commented Apr 13, 2016

Sure, I'll give it a shot. I'm having a little trouble running tests on OS X-- everything seems to fail. Any tips?

make test
./config.status config.h
config.status: creating config.h
config.status: config.h is unchanged
TEST test/blame/default-test
| Failed 4 out of 0 test(s)
| [FAIL] blame-default.screen not found
| [FAIL] blame-with-diff.screen not found
| [FAIL] blame-with-diff-no-file-filter.screen not found
| [FAIL] blame-parent-of-74537d9.screen not found

@jonas jonas closed this in 7174bec Apr 13, 2016
@jonas
Copy link
Copy Markdown
Owner

jonas commented Apr 13, 2016

@harish Thanks for the patch. I updated it and added some test cases. Regarding the problem with make test do you have GNU sed installed?

@harish harish deleted the gh-472 branch April 14, 2016 04:49
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.

2 participants