On 6/30/06, Krzysztof Foltman wdev@foltman.com wrote:
ChangeLog:
- EM_GETLINE support (roughly based on code by Thomas Kho, although
there are important changes, for example, it works correctly in RichEdit 1.0 emulation mode) (bug #4305)
- Workaround for wrong window handles (bug# 4452)
- EM_LINELENGTH handles paragraph ends properly (bug #5375)
I'm sending it as one patch, just in case it gets rejected again - trying to split the patch seems to give me some mysterious git problems.
I've just looked at this patch and the previous patch Thomas sent in, and saying that this is roughly based on his patch is putting it lightly. I see that you've added the 1.0 emulation change, but that doesn't warrant you putting your name on his patch. The correct way to go about this would be to wait till his patch gets accepted and then add your code in a new patch, or to make him aware of the problems in his implementation, so he can rewrite it or add the missing feature, but either way, he deserves credit for the code he wrote, and not just saying that your code was based off of his.
James Hawkins wrote:
I've just looked at this patch and the previous patch Thomas sent in, and saying that this is roughly based on his patch is putting it lightly.
The EM_GETLINE code is completely changed (I haven't even seen his "new" implementation - http://www.winehq.org/pipermail/wine-patches/2006-June/028096.html - before sending the patch in, and the old one was the one Alexandre wrote it needed fixing). Actually knowing about that patch would save me an hour or so of unnecessary work.
The testing code is by Thomas and is completely unchanged, too. Yes, I should have emphasized that in the change log. But it wasn't meant to rip him off.
I see that you've added the 1.0 emulation change, but that doesn't warrant you putting your name on his patch.
I guess, let him decide about that.
then add your code in a new patch, or to make him aware of the problems in his implementation,
The fact is:
- he sent the original patch long time ago, and he didn't fix the problems until very recently (because Alexandre only told us about the problem after I reminded him about his patch?)
- the mail from Alexandre about the patch being wrong was explicitly addressed to me, so with lack of reaction from Thomas I assumed I should update the patch
- "my" cleanup was created *independently* from the new (see above) patch from Thomas, and I'm not even sure which version I like more (the new version from Thomas is more compact, and multiplying nCopy by current character size may not be as patch-reject-prone as I initially thought, given Alexandre's stance about nBPC trick)
- the cleanup involved removing lots of the original code and replacing it by my own (only in EM_GETLINE handler, not in the test code) - just compare the old vs "my" version; that's why I decided to add "roughly"
- I think I have enough of "genuinely mine" and rather complex code in riched20 (like initial versions of style management, display, wrapper) to not be suspected of wanting to take credit for a loop doing (essentially) strcat and p=p->next, that I didn't code from scratch, at cost of Thomas :)
missing feature, but either way, he deserves credit for the code he wrote, and not just saying that your code was based off of his.
He obviously does. The previous version of the patch (which I re-sent) HAS his name as the sole author of the changes.
Krzysztof
On 6/30/06, Krzysztof Foltman wdev@foltman.com wrote:
James Hawkins wrote:
I've just looked at this patch and the previous patch Thomas sent in, and saying that this is roughly based on his patch is putting it lightly.
missing feature, but either way, he deserves credit for the code he wrote, and not just saying that your code was based off of his.
He obviously does. The previous version of the patch (which I re-sent) HAS his name as the sole author of the changes.
I'm not accusing you of intentionally ripping Thomas off, only that more credit is due than was given, and I gave possible solutions to this problem. The previous version you sent in did have his name, which is perfectly legit, but this version doesn't. You can either add his name as well as yours to the changelog, or ask him to resend the tests separately.
James Hawkins wrote:
this problem. The previous version you sent in did have his name, which is perfectly legit, but this version doesn't. You can either add his name as well as yours to the changelog, or ask him to resend the tests separately.
So, the current plan is to: - See if the "new Thomas patch" gets accepted during the nearest 2 weeks - If yes, rewrite the 1.0 fix and it send separately - it's totally incompatible with that patch - If not, resubmit the EM_GETLINE stuff, with proper credits this time, maybe as two separate patches (implementation and unit test) - Move EM_LINELENGTH fix and editor pointer check into separate patches (as soon as they're OK so I don't have to split them over and over)
Does that sound reasonable?
Krzysztof
On 6/30/06, Krzysztof Foltman wdev@foltman.com wrote:
So, the current plan is to:
- See if the "new Thomas patch" gets accepted during the nearest 2 weeks
- If yes, rewrite the 1.0 fix and it send separately - it's totally
incompatible with that patch
- If not, resubmit the EM_GETLINE stuff, with proper credits this time,
maybe as two separate patches (implementation and unit test)
- Move EM_LINELENGTH fix and editor pointer check into separate patches
(as soon as they're OK so I don't have to split them over and over)
Does that sound reasonable?
Yea that sounds good, though you should probably just wait a couple days. If it doesn't get accepted before then, it was probably dropped.
On 6/30/06, James Hawkins truiken@gmail.com wrote:
On 6/30/06, Krzysztof Foltman wdev@foltman.com wrote:
So, the current plan is to:
- See if the "new Thomas patch" gets accepted during the nearest 2 weeks
- If yes, rewrite the 1.0 fix and it send separately - it's totally
incompatible with that patch
- If not, resubmit the EM_GETLINE stuff, with proper credits this time,
maybe as two separate patches (implementation and unit test)
- Move EM_LINELENGTH fix and editor pointer check into separate patches
(as soon as they're OK so I don't have to split them over and over)
Does that sound reasonable?
Yea that sounds good, though you should probably just wait a couple days. If it doesn't get accepted before then, it was probably dropped.
I meant to say, "wait a couple days instead of two weeks".
-
James Hawkins -
Krzysztof Foltman