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

Wrong patch when staged line by line, compared to other alternatives #1416

Open
kulkalkul opened this issue Oct 31, 2022 · 8 comments
Open
Labels
bug Something isn't working help wanted Extra attention is needed nostale immune to stale-bot

Comments

@kulkalkul
Copy link

kulkalkul commented Oct 31, 2022

Describe the bug
Wrong patch when line by line staging compared to using hunks or lazygit or git add -e.

To Reproduce
Steps to reproduce the behavior:

  1. Create a empty repo
  2. Stage & commit this on a file:
struct Earth;

impl Earth {
	fn new() -> Self {
		Self
	}
}

struct Mars;

impl Mars {
	fn new() -> Self {
		Self
	}
}
  1. Update the file with this:
struct Earth;

impl Earth {
	fn new() -> Self {
		Self
	}
	fn into_mars(self) -> Mars {
		Mars
	}
}

struct Mars;

impl Mars {
	fn new() -> Self {
		Self
	}
	fn into_earth(self) -> Earth {
		Earth
	}
}
  1. Use gitui to just stage into_mars and its body
  2. Check the resulted patch with git diff --staged

Expected behavior
I expect it to match other commands and tools.

Screenshots
(source code is different from the example above as I recorded these before creating the issue)
Using gitui: https://asciinema.org/a/dL8rEfpOHRQskpa7wmfVTJDQF
Using lazygit: https://asciinema.org/a/z0iSujDQzZO24wr7qXjKNwTf2

Context (please complete the following information):

  • OS/Distro + Version: Windows 10 22H2
  • GitUI Version: 0.21.0
  • Rust version: 1.66.0-nightly

Additional context
I don't know if this is actually an issue because I don't know much about git patches; but it looks like it corrupts(?) the patch.

@kulkalkul kulkalkul added the bug Something isn't working label Oct 31, 2022
@kulkalkul
Copy link
Author

kulkalkul commented Oct 31, 2022

It also duplicates some of the lines if you stage removal of \ No newline at end of file:
image

I believe both issues could be related.

@alexmaco
Copy link
Contributor

alexmaco commented Nov 4, 2022

I think this might also be related to the issue of not being able to stage chunks.
Chunks are identified by a struct HunkHeader and its hash, but, in different contexts the diff returned by things like fn get_diff_raw seems to be different, off by 1 or 2 lines. This results in loading one diff, displaying it, but then when trying to stage, not being able to identify which hunk is selected.

I haven't yet found the cause of this discrepancy,

@extrawurst
Copy link
Owner

The line staging and hunk staging use different methods and eventually the manual one for linestaging should be used for the hunk staging.

The line staging is based off code from nodegit

@kulkalkul
Copy link
Author

The line staging and hunk staging use different methods and eventually the manual one for linestaging should be used for the hunk staging.

The line staging is based off code from nodegit

Though line staging seems to be having the buggy behaviour here

@extrawurst
Copy link
Owner

like @alexmaco said hung staging has issues too, what I am saying is: rather than fixing hunk staging based on a weak duck-taped solution we should rather migrate it to the line based staging and make sure it is more solid

@stale
Copy link

stale bot commented May 21, 2023

This issue has been automatically marked as stale because it has not had any activity half a year. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the dormant Marked by stale bot on close label May 21, 2023
@extrawurst extrawurst added help wanted Extra attention is needed nostale immune to stale-bot and removed dormant Marked by stale bot on close labels May 25, 2023
@kulkalkul
Copy link
Author

If no one is actively working on this I can try coming up with a solution; but I have no prior experience with anything related.

Also, this seems related: #1804

@fmorroni
Copy link

Hi any news on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed nostale immune to stale-bot
Projects
None yet
Development

No branches or pull requests

4 participants