issue-#144 inconsistencies with Files.Locator error codes

[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

[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: ..."?

[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.

[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.

[Josef Templ]

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

[Robert]

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.

[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

[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

[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.

[Robert]

"loc = NIL" is not necessarily wrong.

Have you investigated those occurances and regard them as individually suspicious?