Project JEDI - Issue Tracker
Mantis Bugtracker

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0002515 [JEDI VCL] 00 JVCL Components feature N/A 2005-01-16 09:57 2005-08-10 08:13
Reporter Simes View Status public  
Assigned To obones
Priority normal Resolution fixed  
Status resolved   Product Version 3.00 BETA 2
Summary 0002515: Modified TJvFindReplace source to correct issues 2503, 2504, 2513
Description The code below has been tweaked in several places to correct issues 2503, 2504 and 2513, and also contains corrections as previously indicated for 2507, 2508 and 2509.

Passed Whole-Word and Match-Case options to FindInText funtion, which now checks any potential matches for the required qualities. Changed calls to FindInText to pass additional arguments, and removed any code for when FindInText returned inappropriate matches. Modified FindInTextRev to accept additional arguments.

Removed FTop and FLeft members, and used FPosition to hold data.

In ReplaceAll, DoProgress event was only invoked when the matched text position was a multiple of 60. Strange. Now invoked for every replacement.

SetOptions, SetPosition, SetDialogTop and SetDialogLeft didn't work once dialogs had been created. They all set the value of the appropriate member variable, then called UpdateDialogs, but UpdateDialogs called GetOptions, GetPosition, GetDialogTop and GetDialogLeft, which read their value from the dialogs in preference to the updated member variable. Hence, new value is ignored.

Added new member UpdateProperties to update the local properties from the dialogs.
Additional Information
Tags No tags attached.
Attached Files zip file icon JvFindReplace.zip [^] (4,842 bytes) 2005-01-17 11:50
zip file icon JvFindReplaceAnotated.zip [^] (7,554 bytes) 2005-01-22 10:34
zip file icon JvFindReplace2.zip [^] (4,919 bytes) 2005-05-19 02:23
zip file icon TJvFindReplaceTest.zip [^] (11,836 bytes) 2005-05-19 02:23

- Relationships
related to 0002503resolvedobones TJvFindReplace dialog doesn't replace all occurences on ReplaceAll 
related to 0002504resolvedobones TJvFindReplace dialog doesn't replace with whole-word-only option 
related to 0002505resolvedobones TJvFindReplace doesn't make un-found text available to OnNoFound handler 
related to 0002507resolvedremkobonte TJvFindReplace infinite loop when closing form 
related to 0002508resolvedobones TJvFindReplace incorrectly reports number replaced when Whole Word or Match Case options in effect 
related to 0002509resolvedobones TJvFindReplace - ReplaceAll doesn't respect WholeWord only 
related to 0002513resolvedobones TJvFindReplace frWholeWord or frMatchCase prevents valid matches from being found. 

-  Notes
(0006169)
obones (administrator)
2005-01-17 03:18

Can you put the unit as an attached zip file instead of "additional information"?
That would be most helpful.
(0006246)
obones (administrator)
2005-01-22 02:34

Could you do the following things:

1. Get the latest version from CVS and apply your changes into it.
2. Mark those changes with special comments, something like
/// Simes begin
/// Simes end

I have a really hard time figuring out what your changes do.
For instance, why change the FindText method to accept a length instead of ToPos, but still call it with ToPos in other functions?
(0006252)
Simes (reporter)
2005-01-22 07:37

>> For instance, why change the FindText method to accept a length instead of >> ToPos, but still call it with ToPos in other functions?
I didn't change FindInText to accept a length, it already accepted a length. I simply renamed the argument from ToPos to Len, because it holds a length, not a position. It helped my understanding of the code to change the name. I didn't change it anywhere else because I wanted to fix the problems, not just rename things.

I will mark up the changes, and add some explanation.
(0006254)
obones (administrator)
2005-01-22 09:56

> I didn't change FindInText to accept a length, it already accepted a length.
> I simply renamed the argument from ToPos to Len, because it holds a length,
> not a position. It helped my understanding of the code to change the name.
> I didn't change it anywhere else because I wanted to fix the problems, not
> just rename things.

Fair enough. But I would greatly appreciate if everything is consistent. I know that your time is limited, but for clarity's sake, I'd really like names to mean what they mean.

> I will mark up the changes, and add some explanation.

Thanks a lot for that.
(0006257)
Simes (reporter)
2005-01-22 10:38

I couldn't delete the old zip, so uploaded it with a different name.

There are many comments throughout the code, and much code commented out. So it looks a mess. Everything is marked by ///simes

I hope it makes things clearer... but I suspect we'll be having some long conversations!

I'm happy to answer any queries, but would prefer to do it via email rather than through here. Can you get my email address, without me posting it here?

BTW, did you run the sample code I posted with the bug reports?
(0006357)
obones (administrator)
2005-02-03 02:28

Yes I can, expect an email soon.
(0006366)
Simes (reporter)
2005-02-03 13:39

I'm afraid my darling wife mistook your email for spam and deleted it. However, I did get a brief glimpse of it beforehand, but it means I don't have your email address.

Yes, I am willing to help, although like everybody else, I already have too many demands on my time and can't guarantee a quick response to any queries.

How would it work? Would I have direct access to CVS? Or would I post any corrected code here and let one of the big boys check it and then update CVS?

Do JEDI use any module/regression test utilities? I'd hate to introduce more bugs than I fix.
(0006370)
obones (administrator)
2005-02-03 23:53

Copy of the next email follows.

--------------------------------------------------------------------
Hi again.

Let's hope this one does not get classified as spam.
Anyway, here are the answers to your questions and remarks:

I understand you are busy, we all are. As long as a response is provided within, say, a month, that's fine by me.

At first, I would suggest you post your changes as attachments to the issues in Mantis, and a bit further down the track you will be granted CVS access. I'm a bit paranoid with CVS access, due to past bad experiences with reckless and stupid developpers.

Finally, as to the regression test utilies, there is not much done in that area. We have a bunch of unit tests in \dev\DUnit\source but not many people have been working on this. A pity though, as I'm a great fan of test driven development. Well, not full blown TDD, but at least write test functions to be run on the code to do regression testing.
If you feel like writing some, please do, but I can't force you to. It's based on DUnit.

Cheers

Olivier
(0007233)
Simes (reporter)
2005-05-19 00:16

Olivier,

I see this still hasn't made it into JEDI. Have you changed your mind?

(I also spent some time writing module tests - I emailed them to you quite some time ago, hoping for some feedback as to whether they were along the right lines, before I uploaded them here.)
(0007234)
obones (administrator)
2005-05-19 00:29

No I haven't changed my mind, the thing is that I don't have much time to look at this.
These are "big" changes, and I must take the time to assess the consequences of such changes. Indeed, there might be some people relying on the original behaviour that would be annoyed by the new behaviour.
I'm not saying your changes are bad, but I must take everything into account and this requires time, which I've been lacking lately.
I'll put this up for discussion on our newsgroup as well, to see what people have to say about all this.
Thanks again for your help, it is very much appreciated even if it doesn't necessrily show from the delays in answering.
(0007235)
Simes (reporter)
2005-05-19 02:24

I've just uploaded a very slightly modified version of my code, and a module test. I'd like to delete the older zips, but don't have the required privilege.
(0007779)
obones (administrator)
2005-08-10 08:13

Ok, I finally had time to look at this and it's all in CVS.
Should you find any other problems, please post other issues.

- Issue History
Date Modified Username Field Change
2005-01-16 09:57 Simes New Issue
2005-01-17 03:13 obones Relationship added related to 0002503
2005-01-17 03:14 obones Relationship added related to 0002504
2005-01-17 03:14 obones Relationship added related to 0002505
2005-01-17 03:15 obones Relationship added related to 0002507
2005-01-17 03:15 obones Relationship added related to 0002508
2005-01-17 03:15 obones Relationship added related to 0002509
2005-01-17 03:16 obones Relationship added related to 0002513
2005-01-17 03:18 obones Note Added: 0006169
2005-01-17 03:18 obones Status new => feedback
2005-01-17 03:18 obones Additional Information Updated
2005-01-17 11:50 Simes File Added: JvFindReplace.zip
2005-01-19 02:29 obones Status feedback => assigned
2005-01-19 02:29 obones Assigned To => obones
2005-01-22 02:34 obones Note Added: 0006246
2005-01-22 02:34 obones Status assigned => feedback
2005-01-22 07:37 Simes Note Added: 0006252
2005-01-22 09:56 obones Note Added: 0006254
2005-01-22 10:34 Simes File Added: JvFindReplaceAnotated.zip
2005-01-22 10:38 Simes Note Added: 0006257
2005-02-03 02:28 obones Note Added: 0006357
2005-02-03 13:39 Simes Note Added: 0006366
2005-02-03 23:53 obones Note Added: 0006370
2005-05-19 00:16 Simes Note Added: 0007233
2005-05-19 00:29 obones Note Added: 0007234
2005-05-19 02:23 Simes File Added: JvFindReplace2.zip
2005-05-19 02:23 Simes File Added: TJvFindReplaceTest.zip
2005-05-19 02:24 Simes Note Added: 0007235
2005-08-10 08:13 obones Status feedback => resolved
2005-08-10 08:13 obones Resolution open => fixed
2005-08-10 08:13 obones Note Added: 0007779


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker