Page 1 of 2
issue-#144 inconsistencies with Files.Locator error codes
Posted: Fri Dec 16, 2016 10:11 am
by Josef Templ
I have created a bug fix issue for the inconsistencies with the Files.Locator error codes.
See the issue at
http://redmine.blackboxframework.org/issues/144.
See the diffs at
http://redmine.blackboxframework.org/pr ... 38f8d9c26c.
Most fixes are trivial, some are a bit subtle (e.g. StdCoder).
Please review carefully.
I have expanded the docu of "This" slightly with the hope of avoiding confusion in the future.
- Josef
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Fri Dec 16, 2016 11:16 am
by Robert
You are right; some changes are a bit confusing.
Would it be simpler (& correct) to change the signature of StdCoder.GetFile to "OUT loc: ..."?
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Fri Dec 16, 2016 11:58 am
by Robert
Regarding the Docu for Directory.This():
1 - "Typically returns the BlackBox directory." Does it ever do anything different? Is that useful? If it does I think the Docu should explain what, and when.
2 - Can we clarify what this means in a Client/Server installation. Is it the Server or Work directory (to use the terms used in the System User Manual )?
3 - What happens when one is running a Packed version? Does that want some additional explanation?
Of course I would find some Windows specific examples useful (eg is a final '\' or '/' in path ignored?), but I understand why the Docu is deliberately platform agnostic.
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Fri Dec 16, 2016 12:14 pm
by Robert
Code: Select all
subloc := root.This(subinfo.name);
IF subloc # NIL THEN
looks suspicious in DevRBrowser.AddFiles.
Also ObxFileTree.Update.
Also ObxLinks.Open.
Also DevLinkChk.ReadLoc - the ASSERT.
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Fri Dec 16, 2016 2:42 pm
by Josef Templ
Robert wrote:
looks suspicious in DevRBrowser.AddFiles.
Also ObxFileTree.Update.
Also ObxLinks.Open.
Also DevLinkChk.ReadLoc - the ASSERT.
Fixed. See diffs at
http://redmine.blackboxframework.org/pr ... 534fb20bc2.
I noticed identical PathToLoc procedures in DevRBrowser, ObxLinks, and DevLinkChk,
and ObxFileTree.ThisLocator is of the same kind.
Can't they be removed all together?
I mean what they do is they parse a path into name pieces and build a locator piece by piece.
Isn't such a locator returned with Files.dir.This(path) anyway?
ad StdCoder.GetFile: improvements such as OUT may be possible but IN/OUT is not used in this module anywhere else.
Using OUT for the loc and name should be possible but symmetrically using IN for path is not, or at least
it is not easy because path is truncated in case of an error. I didn't want to go into all the details of
error handling so I just kept it as it is, with one improvement: name is set to "" at the beginning, because
this is not guaranteed in all error cases.
- Josef
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Fri Dec 16, 2016 3:25 pm
by Robert
Josef Templ wrote:I noticed identical PathToLoc procedures in DevRBrowser, ObxLinks, and DevLinkChk,
and ObxFileTree.ThisLocator is of the same kind.
Can't they be removed all together?
I mean what they do is they parse a path into name pieces and build a locator piece by piece.
Isn't such a locator returned with Files.dir.This(path) anyway?
Probably!
What I noticed this morning was the very similar
HostPackedFiles.GetLoc.It has extensive error reporting, and the
option to create any missing paths. I thought it would be a generally useful procedure if exported and documented - maybe inside a non-Host wrapper.
There is definately scope for rationalisation in this area.
PS - I know that OUT & IN are generally underused; I mention this specific OUT as it would have helped me track the
loc logic today.
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Sat Dec 17, 2016 3:00 am
by DGDanforth
There are still many instances of loc = NIL and loc # NIL that have not been handled.
I will post a list of those modules in a few minutes.
-Doug
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Sat Dec 17, 2016 4:10 am
by DGDanforth
Here is what I did.
Searched all Mod files of BB1.7.1-a1 for instances of
loc = NIL
or
loc # NIL
I then compared the generated list to Josef's first dif listing and then to his second dif listing.
I found 60 instances which were not fixed. Here is the list
Location
Dev/Mod/Cmds.odc
loc # NIL [1]
Dev/Mod/ComInterfaceGen.odc
loc # NIL [1]
Dev/Mod/Compiler.odc
loc # NIL [1]
Dev/Mod/LinkChk.odc
loc # NIL [3] 2 missed
Dev/Mod/RBrowser.odc
loc # NIL [2] 1 missed IF loc # NIL
Host/Mod/Cmds.odc
loc = NIL [1]
loc # NIL [4]
Host/Mod/Dialog.odc
loc # NIL [6]
Host/Mod/Files.odc
loc # NIL [6]
Host/Mod/PackedFiles.odc
loc = NIL [2]
loc # NIL [7]
Host/Mod/Windows.odc
loc # NIL [2]
Obx/Mod/Ascii.odc
loc = NIL [2]
Obx/Mod/MMerge.odc
loc # NIL [1]
Obx/Mod/Orders.odc
loc = NIL [1]
loc # NIL [2]
Std/Mod/Api.odc
loc # NIL [5] 1 missed ASSERT(loc # NIL, 100);
Std/Mod/Coder.odc
loc # NIL [6] 4 missed
Std/Mod/Dialog.odc
loc = NIL [2] 1 missed
loc # NIL [5] 2 missed
Std/Mod/MenuTool.odc
loc = NIL [4]
loc # NIL [2] 1 missed
System/Mod/Converters.odc
loc # NIL [2]
System/Mod/Dialog.odc
loc # NIL [2]
System/Mod/Views.odc
loc = NIL [2]
loc # NIL [4]
System/Mod/Windows.odc
loc # NIL [2]
60 not fixed
-Doug
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Sat Dec 17, 2016 5:12 am
by DGDanforth
Found another one. Just to be careful I did a search for ".This(" and checked the name of the output
to see if some name other than "loc" is used.
In Host/Mod/PackedFiles.odc I found "roots" was used and and a test for "roots = NIL" is done
BUT on initialization roots is set to NIL so that test may be OK.
Also "curloc" is used but its use seems to be OK.
Re: issue-#144 inconsistencies with Files.Locator error code
Posted: Sat Dec 17, 2016 9:04 am
by Robert
"loc = NIL" is not necessarily wrong.
Have you investigated those occurances and regard them as individually suspicious?