Open ywwry66 opened 1 month ago
This PR only contains repeated removal of with-ivy-window
, so I assume it could potentially be treated as legally insignificant?
A regular series of repeated changes, such as renaming a symbol, is not legally significant even if the symbol has to be renamed in many places
One thing this PR does not cover is the outdated description of the usage of with-ivy-window
in Section 7.3 of the user manual. But those changes would probably require the copyright assignment.
Its usage was subsequently replaced by
(select-window (ivy--get-window ivy-last))
in 1c09e99.
Cross-referencing #639 and #665 for posterity.
Its usage was subsequently replaced by
(select-window (ivy--get-window ivy-last))
in 1c09e99.
That commit says:
So now we use only the first part of `with-ivy-window', which is (select-window (ivy--get-window ivy-last)).
But in fact, the first part of with-ivy-window
is (select-window ... 'norecord)
, i.e. do not record this temporary window switch.
I'm inclined to bring back the norecord argument. Do you foresee any issues with that?
Either way, thanks for the careful analysis so far! As you can tell I'm still double checking the history and details of this part of the code.
This PR only contains repeated removal of
with-ivy-window
, so I assume it could potentially be treated as legally insignificant?
Yes, that's my understanding.
One thing this PR does not cover is the outdated description of the usage of
with-ivy-window
in Section 7.3 of the user manual.
I can take care of these changes...
But those changes would probably require the copyright assignment.
...unless you prefer to get started on the CA process?
This PR removes all
with-ivy-window
usages in actions inswiper.el
What about these two?
https://github.com/abo-abo/swiper/blob/2a25a6fb5b081cb141c5eccac8ee58ab1feeb747/swiper.el#L239-L245
I'm inclined to bring back the norecord argument. Do you foresee any issues with that?
I don't see potential issues from my perspective. Maybe you can push a separate commit to this PR?
...unless you prefer to get started on the CA process?
I think I would avoid the lengthy paperwork unless absolutely necessary. So I will leave this part to you, thanks!
What about these two?
These were missed. Thanks for the quick review and I will update the PR.
This usage might still be necessary because of https://github.com/abo-abo/swiper/commit/3656dfe474ec5a1770b12a894a8b4ef52a1cc0e4?
https://github.com/abo-abo/swiper/blob/2a25a6fb5b081cb141c5eccac8ee58ab1feeb747/swiper.el#L239-L245
The
with-ivy-window
wrapper became redundant in actions after https://github.com/abo-abo/swiper/commit/55a90c919411b71f50713e8fa52ae23021ded3bc. Its usage was subsequently replaced by(select-window (ivy--get-window ivy-last))
in https://github.com/abo-abo/swiper/commit/1c09e99153b36626fcb3d272973e7ef3cb5ee679. Its obselote status was also confirmed by abo-abo in https://github.com/abo-abo/swiper/issues/1727#issuecomment-418703805.This PR removes all
with-ivy-window
usages in actions inswiper.el
andcounsel.el
, which cleans up the code and ensures the fix in https://github.com/abo-abo/swiper/commit/1c09e99153b36626fcb3d272973e7ef3cb5ee679 is correctly applied to all actions.