View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0005716 | JEDI VCL | 00 JVCL Components | public | 2011-11-21 15:36 | 2012-09-10 14:15 |
| Reporter | eje91 | Assigned To | Arioch | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.45 | ||||
| Target Version | Fixed in Version | 3.46 | |||
| Summary | 0005716: TJvIPAddress.AddressValues not updating at runtime | ||||
| Description | The TJvIPAddress properties of AddressValues.Address as well as AddressValues.Value1, AddressValues.Value2, AddressValues.Value3, and AddressValues.Value4 do not change at runtime. You can successfully set the values at design time, but they do not change at runtime as the user types in the field. AddressValues.Text does successfully get updated as the user types. I tested this in both Win32 and Win64 applications under Delphi XE2. | ||||
| Additional Information | I posted this previously under ID 0005713 and am re-posting it while adding sample code. Sorry for the redundant post, but I could not figure out how to add code after-the-fact to an old report. | ||||
| Tags | No tags attached. | ||||
|
2011-11-21 15:36
|
TJvIPAddressDemo.ZIP (2,799,881 bytes) |
|
|
this component told to be very buggy in Windows, Maybe Borland had good reasoning to avoid wrapping it. http://www.vbfrance.com/codes/IPADDRESS-CLASS_52546.aspx |
|
|
Personally i dislike this wdget. It is build of some black voodoo magic. What are those edits ? why ? when used ? Weird! Maybe they used to fix bugs in Win2000 // WinXP, dunno. Black arts disguised as code. TJvIPAddress = class(TJvCustomControl) private FEditControls: array [0..3] of TJvIPEditControlHelper; |
|
|
i also think that TJvIPAddressValues.Address should be made private. It is just clone of owner's TJvIPAddress.Address, but made made quite non-optimal. Probably TJvIPAddressValues should better be deleted and its functionality (splitting cardinal into 4 bytes) merged into main component. With all respect to Peter Thrnqvist, i think it was not his brightest day, when he devised this subclass. ---- AS of today - it was done and changed code attached |
|
|
Many thanks to Arioch for the all the time spent on this TJvIPAddress component. I greatly appreciate it! |
|
|
I'm sorry, but I did not actually try the various code changes since I'm certainly no expert. When today's daily JVCL download failed to solve the problem, I utilized the tip suggested in your first reply. That trick of assigning the TJvIPAddress.Text property to a string and thus causing the TJvIPAddress.AddressValues properties to take on the correct values worked well for me, although it is of course not a "clean" fix. This revised code based on your trick functions properly: procedure TForm1.btnIPInfoClick(Sender: TObject); var strDummy : String; begin with JvIPAddress do begin lblJvIPAddressAddress.Caption := IntToStr(Address); { The following is line is a workaround for a problem in which JvIPAddress.AddressValues fields do not get updated even though the user has typed into the JvIPAddress control. After assigning to JvIPAddress.Text to a string, for some reason the JvIPAddress.AddressValues fields have the proper value. } strDummy := Text; lblJvIPAddressValuesAddress.Caption := IntToStr(AddressValues.Address); lblJvIPAddressValuesValue1.Caption := IntToStr(AddressValues.Value1); lblJvIPAddressValuesValue2.Caption := IntToStr(AddressValues.Value2); lblJvIPAddressValuesValue3.Caption := IntToStr(AddressValues.Value3); lblJvIPAddressValuesValue4.Caption := IntToStr(AddressValues.Value4); end { with..do } end; I'll keep the trick code until a future version of JVCL that eliminates the need for it. |
|
|
Will do...I'll follow up after testing. |
|
|
> That trick of assigning the TJvIPAddress.Text property to a string Basically it was just reading the string. To me most dumb way would just to reuse next assignment target. But if to play that seriously, better than introducing dummy var would be just somthing like: " if Text>'' then ;" - same effect of reading property without stretching mind span for yet another var |
|
|
Thank you for the step-by-step directions. I followed them exactly, added JvComCtrls.pas to the project so it would be recompiled, and rebuilt the project. Everything now works without need for the workaround of accessing the .Text property. No adverse side-effects were seen. Again, thank you for your expert guidance. |
|
|
too late <grin smile> |
|
|
Please Ed, test this last one too. .AddressValue[1] is to be used instead AddressValues[1], but rest for you should seems the same, apart that DFM now has much less garbage and code about splitting value to parts is much streamlined. |
|
|
Hello, Arioch. I will test this next week. Sorry for delay! Ed |
|
|
Arioch, Hello. I apologize for the delay. Not enough hours in a day... I recompiled the package in the following directory as it exists on my particular system: C:\Jedi\Jvcl\packages\d16_x64\JvStdCtrlsDesign.dpk Everything works perfectly. Thank you! Ed Again, I apologize for the delay. Not enough hours in a day... Ed |
|
2011-12-16 22:11
|
JvDeprecated.pas (2,183 bytes) |
|
|
I see, New Year's eve :-) But maybe at week-end :-P Now, more serious: 1) There should be TWO packages i think, for 32 bit and for 64 bit code. Your application, is it 32-bit or 64-bit ? And... the JvStdCtrlsDesign.dpk is design-time package, it should not be 64-bit at all (Delphi itself is 32-bit and so can only use 32-bit design-time packages) Truly, the fact that u found it in 64 folder seems to be installer bug to me. I have the only file and it is C:\Delphi\Libs\JediVCL\jvcl\packages\d16\JvStdCtrlsDesign.dpk 2) re-compiling design-time package would only work after u save those two units (one is new, another is overwriting the JVCL's one) and edit the third. ~~~~~~ self-quoting ~~~~~~~~ C:\Delphi\Libs\JediVCL\jvcl\design\JvIP4AddressProp.pas - attached C:\Delphi\Libs\JediVCL\jvcl\design\JvStdCtrlsReg.pas * should use unit above * should have following as last line of Register procedure "RegisterPropertyEditor(TypeInfo(LongWord), TJvIPAddress, 'Address', TJvIP4AddressProperty);" C:\Delphi\Libs\JediVCL\jvcl\run\JvComCtrls.pas - attached ~~~~~~~~~~~~~~ Did you do the above ? 3) Design-time unit recompiling is just a cherry on top of runtime internals simplification and streamlining. You can do without it, you just would find that Object Inspector no more have byte-values of IPAddress, so this patching plus recompilation would bring them back... to a kind ;-) And again, it should be 32-bit design-time package to be recompiled and used in Delphi IDE. After that you should see difference in Object Inspector, did you ? 4) last added unit, JvDeprecated, probably may allow you to just use streamlined component without changing your code. It would be interesting to see if i succeeded in that too. When have time, of course, JvDeprecated should be disposed of and the very source of your project be reworked instead. Really hoping to hear from you, sorry :-) |
|
|
My mistake, Arioch! Since my application is intended to be 64-bit, I looked in C:\Jedi\Jvcl\packages\d16_x64\JvStdCtrlsDesign.dpk. But, of course there should not have been a design package there, and there was not. It existed only at the 32-bit path as you expected: C:\Jedi\Jvcl\packages\d16\JvStdCtrlsDesign.dpk I won't worry about JvDeprecated, but I will make time to test with the other attached files and verify proper Object Inspector behavior. Ed |
|
|
those files would change the source semantics for a bit. Like i described: ~~~~~~~~~~~~~~ .AddressValue[1] is to be used instead .AddressValues.Value1, but rest for you should seems the same, apart that DFM now has much less garbage and code about splitting value to parts is much streamlined. ~~~~~~~~~~~~~~ So the sample would just not compile after that change. The fix is simplistic, but may be tedious when short of time amnd many code places in the project. JvDeprecated (just the fact of "using" it) should bring old semantics back until developer (you) have time for proper fix. Okay, have a nice weekend, hope to hear back the results. |
|
|
i like the code it makes. Pure insanity.... -------- Main.pas.49: lblJvIPAddressValuesAddress.Caption := IntToStr(AddressValues.Address); 00563B20 8B45F8 mov eax,[ebp-$08] 00563B23 8945E0 mov [ebp-$20],eax 00563B26 8B45E0 mov eax,[ebp-$20] 00563B29 8945F4 mov [ebp-$0c],eax 00563B2C 8B45F4 mov eax,[ebp-$0c] ... Main.pas.50: lblJvIPAddressValuesValue1.Caption := IntToStr(AddressValues.Value1); 00563B52 8B45F8 mov eax,[ebp-$08] 00563B55 8945DC mov [ebp-$24],eax 00563B58 8B45DC mov eax,[ebp-$24] 00563B5B 8945F0 mov [ebp-$10],eax 00563B5E 8B45F0 mov eax,[ebp-$10] 00563B61 8945D4 mov [ebp-$2c],eax 00563B64 8B45D4 mov eax,[ebp-$2c] 00563B67 BA04000000 mov edx,$00000004 .... ------------ |
|
2011-12-16 23:57
|
main.pas (2,300 bytes) |
|
|
Updating demo for changes |
|
2011-12-16 23:57
|
main.dfm (3,148 bytes) |
|
|
Sorry, Arioch. I was tied up in meetings today but now am free to test this afternoon. Ed |
|
|
Arioch, Unexpected commitments arose here at work Tuesday afternoon...thought I was free to test but it did not tunr out that way. I have the best intentions of making this Priority 0000001 early Wednesday AM...we'll see how that goes! Ed |
|
|
Arioch, Here’s what I did (based on the actual paths my system uses): 1) Copied your JvIP4AddressProp.Pas file to the C:\JEDI\JVCL\Design folder. 2) Edited C:\Jedi\JVCL\Design\JvStdCtrlsReg.Pas so it “uses” JvIP4AddressProp. 3) Edited C:\Jedi\JVCL\Design\JvStdCtrlsReg.Pas so that the last line of the Register procedure is "RegisterPropertyEditor(TypeInfo(LongWord), TJvIPAddress, 'Address', TJvIP4AddressProperty);" 4) Overwrote the existing C:\Jedi\JVCL\Run\JvComCtrls.Pas with your modified version. 5) Opened C:\Jedi\Jvcl\packages\d16\JvStdCtrlsDesign.Dpk and attempted to build. 6) Since Delphi reported an error “Cannot resolve unit name JvIP4AddressProp”, I copied your JvIP4AddressProp.Pas file to the C:\JEDI\JVCL\Run folder. Not sure if that was a good idea, but it cleared the error... 7) Delphi reported an error in JvIP4AddressProp.Pas stating that TJvIP4AddressComponentIndex was an undeclared identifier. How can that be when TJvIP4AddressComponentIndex is declared in JvComCtrls which is listed in the “uses” clause of JvIP4AddressProp? Ed |
|
|
I should have mentioned that without copying the JvIP4AddressProp.Pas file to the C:\JEDI\JVCL\Run folder and having JvIP4AddressProp.Pas only in C:\JEDI\JVCL\Design, the error reported is that JvIP4AddressProp.Dcu is not found. That gets reported in the “uses” clause where I added JvIP4AddressProp. |
|
|
I have now deleted C:\JEDI\JVCL\Run\JvIP4AddressProp.Pas since you never intended for me to copy the file there in the first place. It now exists only as C:\JEDI\JVCL\Design\JvIP4AddressProp.Pas as per your instructions. Of course I have no JvIP4AddressProp.Dcu. |
|
|
AS. Since you do not use JvDeprecated and since you replaced JvComCtrls, then your project should no more compile, if you did not changed it like i changed the demo ? > Since Delphi reported an error “Cannot resolve unit name JvIP4AddressProp” hmm... i thought search paths are set for design-time packages to include Design folder... Open C:\Jedi\Jvcl\packages\d16\JvStdCtrlsDesign.Dpk and go menu Project / Add to project and point to JvIP4AddressProp Alternatively u can open the package, go Project / Options, go Delphi / Compiler / Search Path and add ";..\..\Design" to the list Both should do, though i do not know which would be more persistent in light of JVCL updates... PS: sorry for late reply, had the busy day --- > How can that be when TJvIP4AddressComponentIndex is declared in JvComCtrls which is listed in the “uses” clause of JvIP4AddressProp? i'm fool you should update proper BPL and DCP files that contain it... In other words, C:\Jedi\Jvcl\packages\d16\JvStdCtrls.Dpk should be recompiled before attempt at recompiling JvStdCtrlsDesign.Dpk, which is stacked on top of the former If your application "uses runtime packages" in Project Options, rather than making one elephant exe with all units inside, then it would also explain why it is compilable still - Delphi just took old compiled version from JvStdCtrls runtime package and did no use of my edited sources :-) |
|
|
Modern Delphi sucks. In D5 '1999 u just open JvStdCtrlsDesign.Dpk, open Requires branch, double-click JvStdCtrls.DCP - and u have both packages open and can recompile any needed in any order. In XE2 u only can close one package and then open another. Or create project group for those two packages. Pitiful and unneeded complication of things :-( |
|
|
Arioch, Thanks as always for your help. We’re getting somewhere now... First, let me tell you that I wiped out the test project’s source code that referred to the deprecated usage. I just want to use you new, improved code. Backwards compatibility is no important to me. C:\Jedi\Jvcl\packages\d16\JvStdCtrls.Dpk was opened and recompiled as per your latest suggestion. That succeeded. Next, C:\Jedi\Jvcl\packages\d16\JvStdCtrlsDesign.Dpk was opened, JvIP4AddressProp was added to the project, and the package was recompiled. That also succeeded. The above steps almost let the Win32 version of the test project compile. All of the AddressValue[1], AddressValue[2], etc. references generated “Undeclared identifier” errors. I then searched my drive for JvIP4AddressProp.Dcu and noticed that \Jedi\JVCL\lib\16 was where it was stored. Changing the project’s search path to include \Jedi\JVCL\lib\16 solved the problem. The Win32 version compiled and ran perfectly. Note that \Jedi\JVCL\lib\16 is not a typo. It seems like it should have been \Jedi\JVCL\lib\d16 with a d before the 16. I do indeed have a \Jedi\JVCL\lib\d16 folder containing Win32 and Win64 subfolders, but there is no JvIP4AddressProp.Dcu within them. JvIP4AddressProp.Dcu exists only at \Jedi\JVCL\lib\16, and \Jedi\JVCL\lib\16 has no Win32 and Win64 subfolders. Does that make any sense to you? I cannot get the Win64 version to compile at all. That is my ultimate goal and is, in fact, the only reason I am migrating away from an old Win32-only I.P. address editor component to this JEDI editor. Did you try the Win64 compilation? Ed |
|
|
> JvIP4AddressProp.Dcu is only used by IDE, not by the project itself u'd better searched for stale JvComCtrls.DCU :-) > to include \Jedi\JVCL\lib\16 solved the problem that was where was stored JvComCtrls.DCU you could also check "use runtime packages" or you could include into search path \Jedi\JVCL\run and .....\common to allow it recompile this last thing is still recommended for Code Completion to work, Error Insigt and other IDE Structure Panel related features. i did not try x64 ever are compilation broken for that control or for some other code ? > Note that \Jedi\JVCL\lib\16 is not a typo. i have it too what are timestamps there, i guess it was one of those DPKs rebuilds that made them probably some of DPKs have directory settings wrong for Output path |
|
|
Changing the project search path to C:\Jedi\JVCL\run;C:\Jedi\JVCL\common\Jedi allowed both the Win32 and Win64 versions to compile and run! The timestamps in that that unexpected \Jedi\JVCL\lib\16 dir are all today from when I rebuilt the files. When you mentioned that some of the DPKs probably have the directory settings wrong for Output path, is that something directly related your recent TJvIPAddress work, or is it a broader JEDI code issue in general? |
|
|
broader all my additions were told youto repeat step by step since u did not changed paths to \16, neither did i |
|
|
> C:\Jedi\JVCL\run;C:\Jedi\JVCL\common\Jedi this last \Jedi is a bit too much C:\Jedi\JVCL\common is proper thing to do jedi subfolder is already accounted for within C:\Delphi\Libs\JediVCL\jvcl\common\jvcl.inc |
|
|
i hope you'll heavily test the result in both 32-bit and 664-bit mode. how's new layout of IPAddress in Object Inspector ? instead of adjusting all DPK paths, i believe u may run C:\Delphi\Libs\JediVCL\jvcl\makemodified.bat to make newer DCUs in \d16 subfolders But before this you'd add JvIP4AddressProp to Contains section of C:\Delphi\Libs\JediVCL\jvcl\packages\xml\JvStdCtrlsDesign-D.xml |
|
|
Thanks for the search path tip. It is just C:\JEDI\JVCL\Run;C:\JEDI\JVCL\Common. I just ran the Win64 version, and it works great. In my earlier update, it had compiled successfully on a Win32 development machine, but the executable had not yet been run. It has no trouble. The only Object Inspector trouble I've seen is when trying to type an address in the main Address line. Try typing 192.168.168.17, and the Address[1] value will always be 0. However, type that in Text line, and it works fine. Do you like ShortInt for Address[n], or would Byte be better? Will you be releasing this now as a full-fledged JEDI update? |
|
|
the wrong paths come from C:\Delphi\Libs\JediVCL\jvcl\packages\d16\template.dproj and given that file makes no difference for x86 and x64 and other targets (non-Windows) - it hardly can be modified to both use d16\win32 and d16\win64 folders... |
|
|
i cannot even commit this into SVN i think we'd wait for JVCL team to come out of New Year rush when they'd be able to asses all the commited patches in tracker, and there are a number of those. I feel they are in the state liek yours, they only can think about the particular lines (and bugs) they do work themselves with. Hope it would be easier time after NY |
|
|
> in the main Address line. Try typing 192.168.168.17, and the Address[1] value will always be 0 would try it, definitely a bug and should not happen (and you're already talking about release :-) ) > Do you like ShortInt for Address[n], or would Byte be better? Since Address is LongWord/Cardinal/UInt32, i think address components should be unsigned too. And do they have any reason to be signed ? Do you want address like -65.-41.-41.17 instead in your example ? Microsoft thinks the same: http://msdn.microsoft.com/en-us/library/windows/desktop/ff485954(v=VS.85).aspx Though the lack of Delphi unsigned support in StrToInt and related functions annoys me (see 0005742). |
|
|
You can see ? they just CAN NOT act differently. It either sets 'as Address' or 'as Text' and can do nothing other. OR look at below and tell me if i made some stupid error and cannot see it. procedure TJvIP4AddressProperty.SetValue(const Value: string); var tmp: Cardinal; begin if TryStrToUInt(Value, tmp) then SetOrdValue(tmp) else if GetComponent(0) <> nil then if GetComponent(0) is TJvIPAddress then begin TJvIPAddress(GetComponent(0)).Text := Value; Modified; end; end; procedure TJvIP4AddressAsTextProperty.SetValue(const Value: string); begin if GetComponent(0) <> nil then if GetComponent(0) is TJvIPAddress then begin TJvIPAddress(GetComponent(0)).Text := Value; Modified; end; end; procedure TJvIP4AddressAsNumberProperty.SetValue(const Value: string); begin SetOrdValue(StrToUInt(Value)); end; ------- Though that made me do one more small (just one added line) change in JvComCtrls.pas - consider doing the same if you like it. procedure TJvIPAddress.UpdateValuesFromString(S: string); begin S := Trim(s); // add this AddressValue[1] := StrToIntDef(StrToken(S, '.'), 0); AddressValue[2] := StrToIntDef(StrToken(S, '.'), 0); AddressValue[3] := StrToIntDef(StrToken(S, '.'), 0); AddressValue[4] := StrToIntDef(S, 0); end; |
|
|
Where the Object Inspector lists Address 0 = 0.0.0.0 I click on the 2nd of those 5 zeroes (the 1st of the 4-part address), hit delete, and type 192. That should change it to say Address 0 = 192.0.0.0 but it stays Address 0 = 0.0.0.0. If I click on the 3rd of those 5 zeroes, hit delete, and type 168, it changes to say Address 11010048 = 0.168.0.0 as you would expect. Only the first part of the 4-part address fails to work; the last few are all fine. This can be reproduced every time. Adding S := Trim(s) to procedure TJvIPAddress.UpdateValuesFromString(S: string) did not help. |
|
|
u can not set both integer and x.y.z.w - you should either set one or another And basically what u try to do is assigning value like "10 = 14" - so what should component do, believe 1st part or 2nd part of contradicting value ? that limitation made me add As Text and As Address sub-fields, which were not in initial upload. > Only the first part That's because the control itself splits the string over dots (as u can see in UpdateValuesFromString above), so it tries to set 1st component to "11010048 = 0" which obviously fails to be an integer. if you enter "11010048 = 0.168.0.0 " into As Text field - it would act the same. Maybe i should just disable editing that field at all ? or should i make it abort if field contains any characters but dots and numbers ? you opinion ? |
|
|
Yes...blocking editing of the main Address line and forcing the use of the individual fields would be a perfect solution. When I asked about Byte versus ShortInt for Address[n], I didn't mean to imply that I needed signed numbers there. Have a look at the code...ShortInt is currently being used, right? I was just wondering whether you wanted to change it to Byte. |
|
|
function TJvIP4AddressProperty.GetAttributes: TPropertyAttributes; begin Result := [paMultiSelect, paSubProperties, paRevertable, paVolatileSubProperties, paDisplayReadOnly]; if GetPropInfo.SetProc = nil then Result := Result + [paReadOnly, paDisplayReadOnly]; end; ---- Where exactly i use ShortInt ? If somewhere around StrToInt - it can't be easily helped, Delphi does not have string -> unsigned integer conversion. And i do not see it as big deal. No compilation warnings and control does its work to my and your tests. |
|
|
That is my fault. I had moused over one of the AddressValue[n] identifiers in my test project and saw Delphi report ShortInt. It seemed odd that it was not Byte. Now that you've asked where ShortInt was used, I see in JvComCtrls.Pas that there is very clearly the following Byte declaration: property AddressValue[Component: TJvIP4AddressComponentIndex]: Byte Sorry for the mistake... |
|
|
maybe it was n expected type reported instead what about patch above for Object Inspector ? satisfied ? |
|
|
Oh, i understood. Object Inspector just does not have concept of unsigned value - neither byte nor word nor cardinal. Only signed values ShortInt/SmallInt/LongInt it knows about. BTW, did you tried last patch that should make property read-only but visible and copyable ? |
|
|
depending on CPU used, you probably can run and text 64-bit eXEs even on 32-bit development machine, if the CPU itself is capable of 64-bit http://www.virtualbox.org/manual/ch03.html#intro-64bitguests |
|
|
Arioch, Hello. I made your change to function TJvIP4AddressProperty.GetAttributes, but I'm now receiving a compilation error having nothing to do with that specific change. The error when compiling JvIP4AddressProp.pas is: [DCC Fatal Error] JvIP4AddressProp.pas(6): F1026 File not found: 'DesignEditors.dcu' A GREP of my entire system finds no DesignEditors.dcu file. Ed |
|
|
what project exactly and how exactly do u compile ? this unit is with system package (.DCP + .BPL) which exact name i do not recall, but it is in Required section of JvStdCtrlsDesign.Dpk 32-bit and it should be somewhere within search path of course |
|
|
Arioch, I initially addeed JvIP4AddressProp.pas to my project and tried to compile. That was obviously wrong and resulted in the error. This time, though, C:\Jedi\Jvcl\packages\d16\JvStdCtrlsDesign.Dpk was opened with JvIP4AddressProp added to the project. Your TJvIP4AddressProperty.GetAttributes was implemented, and the project was compiled. Next, I opened my usual test project. The Object Inspector did indeed behave differently with respect to the Address property. That property was greyed out to indicate that it could not be changed. Oddly, though, I can still type in that field (it types all gray), and the typed numbers do indeed have an effect. Ed |
|
|
that means i completely fail to read and understand Delphi docs... would you generally prefer it black or gray ? |
|
|
function TJvIP4AddressProperty.GetAttributes: TPropertyAttributes; begin Result := [paReadOnly, paMultiSelect, paSubProperties, paRevertable, paVolatileSubProperties]; end; that wouldkeep it 'black' (dark blue) if you like it gray, then add paDisplayReadOnly to the set above and remove or comment out procedure TJvIP4AddressProperty.SetValue - it would no more be used so let's clean the garbage :-) |
|
|
I like it gray to indicate that it's read-only. Your suggestion of commenting out TJvIP4AddressProperty.SetValue worked. After doing that, you can still try to type in the field, but the changes no longer take effect. It is fine now. Are you ready for full release early next year? |
|
|
Then make it so, as i wrote earlier: function TJvIP4AddressProperty.GetAttributes: TPropertyAttributes; begin Result := [paReadOnly, paDisplayReadOnly, paMultiSelect, paSubProperties, paRevertable, paVolatileSubProperties]; end; About release i wrote earlier in comment (0019234) http://issuetracker.delphi-jedi.org/view.php?id=5716#c19234 I am just a regular (or better say irregular ;-) ) user, hence i do have no more influence upon releases than you. |
|
2011-12-29 22:41
|
JvIP4AddressProp.pas (9,412 bytes) |
|
|
Thanks for all of your hard work, and Happy New Year! |
|
2011-12-29 22:52
|
JvComCtrls.pas (108,659 bytes) |
|
|
Same to you and to JEDI team :-) |
|
|
So i hope someone with grants would review it and make it into SVN |
|
|
from forum: > Right now it's a bit hard to figure out what is to be done from your discussions. i have no SVN upload so could not make a fork. Just had to change files in place. 0) main.* are updated files from test zip example and no interest to you if you don't want to run applied test-case. 1) i refactored the component and removed additional helper class I think attached JvComCtrls.pas is to be diff'ed against SVN version and changes reviewed and copied into SVN Or - if JvComCtrls.pas probably did not changed in SVN from 2011-11-21 - just copied over SVN version. OTOH, if you would not blindly trust me and reporter, then to review changes you anyway have to diff it. 2) introduced design-time property editor which should be added to JvStdCtrlsDesign package The implementation is trivial and probably no point to review it To apply it one needs: 2.0) add JvIP4AddressProp.Pas to JvStdCtrlsDesign package 2.1) Copy JvIP4AddressProp.Pas file to the C:\JEDI\JVCL\Design folder. 2.2) Edit C:\Jedi\JVCL\Design\JvStdCtrlsReg.Pas so it “uses” JvIP4AddressProp. 2.3) Edit C:\Jedi\JVCL\Design\JvStdCtrlsReg.Pas so that the last line of the Register procedure is "RegisterPropertyEditor(TypeInfo(LongWord), TJvIPAddress, 'Address', TJvIP4AddressProperty);" 3) added source-level 'compatibility' helpers into JvDeprecated.pas This i think worth consideration per se. I envision it been placed into JVCL\Run or JVCL\Common and used as framework for most JVCL source-level refactorings. That optionally would be better than just old versions moving units into usupported \Archive, though would only work on newer Delphi versions. Optionally together with CodeLib team consider related enhancement http://issuetracker.delphi-jedi.org/view.php?id=5742 |
|
|
Thank you all for your help. This is now solved in SVN, in a slightly different manner regarding legacy compatibility. As a side note, please respect the JVCL coding style when submitting patches, it makes code inclusion easier. |
|
|
procedure TJvIPAddressRange.SetMaxRange(const Index: Integer; const Value: Byte); 822 var 823 Range: TJvIPAddressMinMax; 824 begin 825 Range := FRange[Index]; 826 Range.Max := Value; 827 if Range.Min > Value then 828 Range.Min := Value; 829 830 Change(Index); 831 end; 832 833 procedure TJvIPAddressRange.SetMinRange(const Index: Integer; const Value: Byte); 834 var 835 Range: TJvIPAddressMinMax; 836 begin 837 Range := FRange[Index]; 838 Range.Min := Value; 839 if Range.Max < Value then 840 Range.Max := Value; 841 842 Change(Index); 843 end; That would not change anything. Perhaps u meant pointers ? var Range: ^TJvIPAddressMinMax; begin Range := @FRange[Index]; Range^.Min := Value; if Range^.Max < Value then Range^.Max := Value; ? But anyway i still believe that local with...do is more readable and turned out to be more reliable. isn't "Range := FRange[Index];" unnecessary and non-effective data pumping ? and anyway it lacks reverse assignment |
|
|
> a slightly different manner regarding legacy compatibility. that is not slightly different - that is completely different. 1) no redundant code cleaned up - that would mess maintaining long term 2) that redundant code would make control flow more complex, placing extra burden onto future debugging sessions 3) no one would read comments in the code for component they use for long. They would not know those properties are deprecated Up to the point why at all new properties ben added ? why ? If they still have to work via old cross-object callbacks, why not just remove them ? My approach provided for centralized deprecation/cleanup mechanism with effective timeline prompting. I remember all the talks about JvArchive when 2.x -> 3.x migration. The centralised approach could do it not that all-at-once way. |
|
|
I'm glad that CodeLib changes commited, then i ask for Property Editor clean-up, let it use commited functions from http://jcl.svn.sourceforge.net/viewvc/jcl/trunk/jcl/source/common/JclSysUtils.pas?r1=3699&r2=3732&pathrev=3732 rather his local copies of them. |
|
|
Example of inconsistency: Let's call procedure TJvIPAddressValues.SetAddress(const AValue: Cardinal) - no event of TJvIPAddressValues is fired. Even when developer may expect OnChange to fire - after all that event by name is supposed to. Now, let's for comparision call procedure TJvIPAddressValues.SetValues(Index: Integer; Value: Byte) 1) event OnChanging is fired for checking Range. Which is hardly necessary delegation 2) procedure TJvIPAddress.SetAddressValue is called, 2.1) checking for Range once again 2.2) updating address 2.3) sending WM_ message to GDI to update control 3) TJvIPAddressValues.OnChange event is fired which calls both function TJvIPAddressValues.GetAddress (reading FAddress) and procedure TJvIPAddress.SetAddress, which probably bails out on (FAddress.Address <> Value) check, otherwise it would once again assign FAddress and ocne againg send GDI message And now someone would debug this all without slightext clue why so complex and searchign for side effects! |
|
|
Minor bug found. Dunno if can manifest itself in regular applications, but still should better be fixed. The marked line should be added. It can be compared with procedure TJvIPAddress.SetAddress procedure TJvIPAddress.SetAddressValue(... .... if AllowChange then begin FAddress.Comps[Component] := Value; PushAddressToWindows; FSaveBlank := False; (*******************) end; |
|
|
Can I get a summary here? |
|
|
a summary of what ? maybe u just give me some SVN sandbox ? 1) we have different opinions about original patch itself. You, probably, think i am too harsh over older Delphi versions, and i think your attempt to keep compatibility at any terms just killed effort to make code simple and consistent. On this point no short summary is probably possible at all. Frankly, in old days we'd had an discussion of forum with many people telling their opinions and mediating. Which tracker is not good place for. 2) Some used functions in design-time module (property editor) by their kind are non-visual and were offered to JCL. Since JCL accepted them, they are now mere duplication and be removed from property editors. 0005742. 3) while reviewing your changes to my changes i found one more bug that was noticed by no one earlier. That is comment (0019706) All those topics are, i think, not interconnected and can be taken one by one. |
|
|
Please stick comments to each issues. Ideally, one bug, one issue. This issue feels more and more like a dumpster and it's not easy to follow track. |
|
|
Few months ago i'd do it. But no i can not add comments to closed issues. Don't know what changed - but... Okay, would try to - re-open it, if it suits better. |
|
|
Ok, this is now resolved. If other problems arise, please create new issues. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2011-11-21 15:36 | eje91 | New Issue | |
| 2011-11-21 15:36 | eje91 | File Added: TJvIPAddressDemo.ZIP | |
| 2011-11-24 13:19 | Arioch | Note Added: 0019129 | |
| 2011-11-24 15:31 | Arioch | Note Added: 0019131 | |
| 2011-11-24 21:26 | Arioch | Note Added: 0019138 | |
| 2011-11-24 21:34 | Arioch | Status | new => assigned |
| 2011-11-28 19:53 | eje91 | Note Added: 0019146 | |
| 2011-11-28 21:19 | eje91 | Note Added: 0019148 | |
| 2011-11-28 21:54 | eje91 | Note Added: 0019150 | |
| 2011-11-28 22:12 | Arioch | Note Added: 0019152 | |
| 2011-11-29 14:58 | Arioch | File Added: JvIP4AddressProp.pas | |
| 2011-11-29 15:00 | Arioch | File Added: JvComCtrls.pas | |
| 2011-11-29 15:00 | eje91 | Note Added: 0019161 | |
| 2011-11-29 15:01 | Arioch | Note Added: 0019162 | |
| 2011-11-30 18:51 | Arioch | Note Added: 0019164 | |
| 2011-11-30 18:53 | Arioch | Status | assigned => resolved |
| 2011-11-30 18:54 | Arioch | Severity | major => minor |
| 2011-11-30 18:54 | Arioch | Status | resolved => feedback |
| 2011-12-02 23:15 | eje91 | Note Added: 0019171 | |
| 2011-12-16 22:03 | eje91 | Note Added: 0019209 | |
| 2011-12-16 22:11 | Arioch | File Added: JvDeprecated.pas | |
| 2011-12-16 22:23 | Arioch | Note Added: 0019211 | |
| 2011-12-16 22:35 | eje91 | Note Added: 0019212 | |
| 2011-12-16 22:40 | Arioch | Note Added: 0019213 | |
| 2011-12-16 23:55 | Arioch | Note Added: 0019214 | |
| 2011-12-16 23:55 | Arioch | File Deleted: JvIP4AddressProp.pas | |
| 2011-12-16 23:56 | Arioch | File Added: JvIP4AddressProp.pas | |
| 2011-12-16 23:57 | Arioch | File Added: main.pas | |
| 2011-12-16 23:57 | Arioch | Note Added: 0019215 | |
| 2011-12-16 23:57 | Arioch | File Added: main.dfm | |
| 2011-12-17 11:08 | Arioch | File Deleted: JvIP4AddressProp.pas | |
| 2011-12-17 11:08 | Arioch | File Added: JvIP4AddressProp.pas | |
| 2011-12-20 19:52 | eje91 | Note Added: 0019219 | |
| 2011-12-20 22:05 | eje91 | Note Added: 0019220 | |
| 2011-12-21 23:12 | eje91 | Note Added: 0019221 | |
| 2011-12-21 23:20 | eje91 | Note Added: 0019222 | |
| 2011-12-21 23:35 | eje91 | Note Added: 0019223 | |
| 2011-12-22 19:11 | Arioch | Note Added: 0019224 | |
| 2011-12-22 19:11 | Arioch | Note Edited: 0019224 | |
| 2011-12-22 19:14 | Arioch | Note Edited: 0019138 | |
| 2011-12-22 19:20 | Arioch | Note Edited: 0019224 | |
| 2011-12-22 19:22 | Arioch | Note Added: 0019225 | |
| 2011-12-22 19:27 | Arioch | Note Edited: 0019224 | |
| 2011-12-22 19:27 | Arioch | Note Edited: 0019225 | |
| 2011-12-22 19:28 | Arioch | Note Edited: 0019224 | |
| 2011-12-22 20:13 | eje91 | Note Added: 0019226 | |
| 2011-12-22 20:37 | Arioch | Note Added: 0019227 | |
| 2011-12-22 21:02 | eje91 | Note Added: 0019228 | |
| 2011-12-22 21:19 | Arioch | Note Added: 0019229 | |
| 2011-12-22 21:25 | Arioch | Note Added: 0019230 | |
| 2011-12-22 21:28 | Arioch | Note Added: 0019231 | |
| 2011-12-22 21:43 | Arioch | Note Edited: 0019230 | |
| 2011-12-22 21:54 | eje91 | Note Added: 0019232 | |
| 2011-12-22 22:01 | Arioch | Note Edited: 0019231 | |
| 2011-12-22 22:07 | Arioch | Note Added: 0019233 | |
| 2011-12-22 22:12 | Arioch | Note Added: 0019234 | |
| 2011-12-22 22:18 | Arioch | Note Added: 0019235 | |
| 2011-12-22 22:20 | Arioch | Note Edited: 0019235 | |
| 2011-12-22 22:31 | Arioch | Note Added: 0019237 | |
| 2011-12-22 22:31 | Arioch | File Deleted: JvComCtrls.pas | |
| 2011-12-22 22:32 | Arioch | File Added: JvComCtrls.pas | |
| 2011-12-22 22:33 | Arioch | Note Edited: 0019235 | |
| 2011-12-22 23:00 | eje91 | Note Added: 0019238 | |
| 2011-12-22 23:44 | Arioch | Note Added: 0019239 | |
| 2011-12-22 23:45 | Arioch | Note Edited: 0019239 | |
| 2011-12-22 23:47 | Arioch | Note Edited: 0019239 | |
| 2011-12-22 23:50 | Arioch | Note Edited: 0019239 | |
| 2011-12-23 00:28 | eje91 | Note Added: 0019240 | |
| 2011-12-23 14:36 | Arioch | Note Added: 0019243 | |
| 2011-12-23 14:36 | Arioch | File Deleted: JvIP4AddressProp.pas | |
| 2011-12-23 14:37 | Arioch | File Added: JvIP4AddressProp.pas | |
| 2011-12-23 15:50 | eje91 | Note Added: 0019244 | |
| 2011-12-23 18:31 | Arioch | Note Added: 0019245 | |
| 2011-12-29 00:19 | Arioch | Note Added: 0019268 | |
| 2011-12-29 00:31 | Arioch | Note Added: 0019269 | |
| 2011-12-29 20:25 | eje91 | Note Added: 0019271 | |
| 2011-12-29 20:33 | Arioch | Note Added: 0019272 | |
| 2011-12-29 20:35 | Arioch | Note Edited: 0019272 | |
| 2011-12-29 20:55 | eje91 | Note Added: 0019273 | |
| 2011-12-29 21:20 | Arioch | Note Added: 0019274 | |
| 2011-12-29 21:23 | Arioch | Note Added: 0019275 | |
| 2011-12-29 22:26 | eje91 | Note Added: 0019276 | |
| 2011-12-29 22:39 | Arioch | Note Added: 0019277 | |
| 2011-12-29 22:40 | Arioch | Note Edited: 0019277 | |
| 2011-12-29 22:41 | Arioch | File Deleted: JvIP4AddressProp.pas | |
| 2011-12-29 22:41 | Arioch | File Added: JvIP4AddressProp.pas | |
| 2011-12-29 22:43 | eje91 | Note Added: 0019278 | |
| 2011-12-29 22:51 | Arioch | File Deleted: JvComCtrls.pas | |
| 2011-12-29 22:52 | Arioch | File Added: JvComCtrls.pas | |
| 2011-12-29 22:52 | Arioch | Note Added: 0019279 | |
| 2011-12-29 22:55 | Arioch | Note Added: 0019280 | |
| 2011-12-29 22:55 | Arioch | Assigned To | => Arioch |
| 2011-12-29 22:55 | Arioch | Status | feedback => confirmed |
| 2012-02-17 19:23 | Arioch | Note Added: 0019430 | |
| 2012-02-24 15:04 | obones | Note Added: 0019534 | |
| 2012-02-24 15:04 | obones | Status | confirmed => resolved |
| 2012-02-24 15:04 | obones | Fixed in Version | => Daily / SVN |
| 2012-02-24 15:04 | obones | Resolution | open => fixed |
| 2012-04-15 16:13 | Arioch | Note Added: 0019702 | |
| 2012-04-15 16:13 | Arioch | Status | resolved => feedback |
| 2012-04-15 16:13 | Arioch | Resolution | fixed => reopened |
| 2012-04-15 16:15 | Arioch | Note Edited: 0019702 | |
| 2012-04-15 16:19 | Arioch | Note Edited: 0019702 | |
| 2012-04-15 16:26 | Arioch | Note Added: 0019703 | |
| 2012-04-15 16:46 | Arioch | Note Added: 0019704 | |
| 2012-04-15 18:30 | Arioch | Note Added: 0019705 | |
| 2012-04-15 18:37 | Arioch | Note Added: 0019706 | |
| 2012-04-15 18:38 | Arioch | Note Edited: 0019706 | |
| 2012-06-11 17:39 | obones | Note Added: 0019874 | |
| 2012-06-11 22:45 | Arioch | Note Added: 0019879 | |
| 2012-06-11 22:45 | Arioch | Note Edited: 0019879 | |
| 2012-06-13 13:33 | obones | Note Added: 0019948 | |
| 2012-06-13 14:48 | Arioch | Note Added: 0019954 | |
| 2012-06-14 12:19 | obones | Note Added: 0019989 | |
| 2012-06-14 12:19 | obones | Status | feedback => resolved |
| 2012-06-14 12:19 | obones | Resolution | reopened => fixed |
| 2012-09-10 14:15 | obones | Fixed in Version | Daily / SVN => 3.46 |
