flyspell-correct
flyspell-correct copied to clipboard
d12frosted
Reimplement gusbrs's Code for Contribution to GNU ELPA
See #99.
~~I'll leave this as a draft until I'm sure it's good copyright wise.~~ The org commit is 13 lines and one line here is still one line under the limit. I'm happy to make changes if there's some corner case I'm missing or if you don't agree with the changes.
I've made it restore the user's original mark-ring (unless they call 'stop). If it was intended as a feature (or if you would prefer it in a separate commit) that the mark was present on the last correction I'm happy to change it back.
Some people like it, I don't use it and sometimes I have to do extra pop to get to the place where I wanted to be. As far as I can tell, some users prefer current behaviour, because sometimes you need to do some modifications around the misspelled word. And IMO for such cases there is stop command.
Perhaps it belongs under a config variable? I'm happy to make such a change in a couple of days (to distance myself from the old implementation again if they're going to have the same featureset) if you want.
flyspell-correct-preserve-mark-ring? Or perhaps flyspell-correct-mark-corrections?
In addition, I would like to invite @gusbrs to review this PR as well.
@d12frosted I'm a little tight at the moment, so I can't really be thorough. But I took a look at the PR and, if I read it correctly, it seems to change current behavior by not setting the mark at all, even when a correction is made (previous lines 424 to 430, which appear not to have an equivalent, and the full restore of the mark ring at the end). Is this understanding correct @Provessor ? (I may be wrong though, please just say it if that's the case). If so, I'd like to argue this is not really a welcome change.
Most of my reasoning behind those changes were actually done in #80 , I still think that those considerations are my best shot of how I think things would work best. Particularly, this comment:
I think a good criterium [sic, criterion] would be to push the mark only when an actual change has been made (to the buffer, or to the dictionary etc) thus removing the overlay. The logic to this would be: if the overlay is intact we can go back there repeating the command, if it's not, a mark would allow us to do so.
From the commit message:
- It no longer touches the user's mark-ring unless they 'stop so the mark is present where corrections began.
That's what I meant above, so it appears to be intentional.
- Less twidling with marks to get the right result, it is now only used to return to the start when necessary.
Well, that depends on a definition of "right result", right? ;-)
Anyway, I'm not here to make fuss about it, specially considering the situation. If the change in behavior is intended and @d12frosted agrees with it, it's all fine. I would personally not really celebrate such change, but I'm just one user, and opinions may vary on the subject. The comments above presume an equivalence in functionality would be desirable, and this is something which is not for me to decide. And I'm really just commenting because it was requested.
Besides that, the only other comment I would have is not really technical, but formal. Considering the reason for this change, wouldn't it be wiser to literally revert my commit, and then perform any changes after that? I'd think this would make things more robust from a legal standpoint. But, well, the usual disclaimer applies: I'm not a lawyer, etc.
But I took a look at the PR and, if I read it correctly, it seems to change current behavior by not setting the mark at all, even when a correction is made (previous lines 424 to 430, which appear not to have an equivalent, and the full restore of the mark ring at the end). Is this understanding correct @Provessor ? (I may be wrong though, please just say it if that's the case). If so, I'd like to argue this is not really a welcome change.
@gusbrs Yes since making this I've read those commits and related discussions. Now I would like to first make this produce as close of a result as possible then anything else like changing this mark behaviour based on a variable can come later.
Well, that depends on a definition of "right result", right? ;-)
Yes I misunderstood the original purpose of changing the mark so much.
Besides that, the only other comment I would have is not really technical, but formal. Considering the reason for this change, wouldn't it be wiser to literally revert my commit, and then perform any changes after that? I'd think this would make things more robust from a legal standpoint. But, well, the usual disclaimer applies: I'm not a lawyer, etc.
That's a good point, I'll see if I know enough about git to make that happen.
I've been rather busy the past few days but I would like to have this resolved soon if other work permits.
I've tried to implement the same behaviour after reverting the commits and ended up with almost identical code. I also can't see a clean alternative without touching too much other code.
@d12frosted If I have to in order to get different code do you have a preference between also adding features or restructuring the other point/mark management code?
@Provessor apologies for missing your question.
If I have to in order to get different code do you have a preference between also adding features or restructuring the other point/mark management code?
No, as long as @gusbrs is happy about it :) @gusbrs understands mark management in this package better than me
No, as long as @gusbrs is happy about it :)
@d12frosted Sorry, but I would not like to be made an arbiter on this. I was happy to make the contribution originally, and to share an opinion on the PR, as I already did. But I think it is not up to me, and that I offered what I could on this. Whatever the merits of the FSF copyright policy, it does have the side-effect of putting some people aside, me among them. It is not a comfortable position to be in, if one wishes to contribute. It is not your fault, of course, but it is what it is. So, please, do what you think is best here. The track record of flyspell-correct makes it great already. ;-)
@gusbrs I totally understand this and no need to appologise. And actually it's me who needs to appologise for letting this project being dragged into a land where your contributions are not welcomed. Not that "free as in freedom" after all. Sorry, Gustavo! And I appreciate your contributions 💯
@d12frosted Well, I have myself concurred it was a good move, when the suggestion was made. I still do. Alas, there are some cons. But it is a trade-off, we should live with it. No reason for you to regret it, or apologize for it. ;-)
I just asked for your understanding in letting me "slip away" of the associated chores, which is a fair request in the circumstances. Thank you for that!
