Ignore fixup or squash commits in fixup_candidates_lines

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested

[guludo]

Fixup or squash commits are already ignored in fixup_candidates_files, do that in fixup_candidates_lines as well.

[keis]

Hey, thanks for the pull request!

I need to think about this one a bit and haven't really had the time but I'll try to write down my thoughts.

First, this doesn't have a comparable effect to what is done in fixup_candidates_file. That function is pretty simple and takes the git log excluding fixups and provides the first result, and crucially still outputs a candidate even if the first commit is a filtered. This in contrast gets the candidates and the filters after.

It's possible the commit range passed to blame could be tweaked to get a "non-fixup" suggestion but that's probably pretty complex. For one it would have to consider changes to line numbers in those fixup commits that are skipped.

As the effect of the filter is pretty much equivalent to git-fixup | grep -v fixup the code could be simplified and avoid that extra rev-list

If you're changing the same line again as a previous fixup in a way it kinda makes sense to think of as a fixup to a fixup, but I guess git rebase doesn't handle that situation. And is filtering the candidate really helping? I imagine in that situation you would get nothing instead.

and a final btw after 5 years maybe it's now time to use --invert-grep #34

Sorry for the big train of thought dump, let me know what you think. If you have any examples of scenarios where you would want this is also good.

[guludo]

Hi, @keis. Thank you for your comments.

In my use case, I had multiple fixes for a single commit (and possibly others for other commits as well). I wanted to do them incrementally, so I could review them before performing the rebase of my feature branch. As such, showing fixup commits as candidates did not have any use for me. While piping the output to a grep commit would be reasonable, for me it wouldn't be so convenient as I am using the menu feature.

[guludo]

I took a second look at the code and, indeed, filtering after is somewhat awkward. Although it might work in some cases, it might not produce the desired effect in other cases.

Maybe a better fix here would be to lookup the original commit for each fixup commit found? That could be done by finding the first commit in the revision range that matches the title without the fixup! or squash! prefix.

[keis]

Hey, sorry for the long delay, responding to this got lost in my TODO list somewhere.

I like the idea of looking up the original commit for fixup commits that would be suggested