Views.Old doesn't impement documentation specs

All except GUI problems

Views.Old doesn't impement documentation specs

Postby adimetrius » Thu Jul 09, 2020 9:04 pm

Colleagues,
Views Documentation wrote:PROCEDURE Old (ask: BOOLEAN; VAR loc: Files.Locator; VAR name: Files.Name;
VAR conv: Converters.Converter): View
conv determines the converter which is used for reading the document. conv = NIL means that no conversion is necessary, i.e., the file format already has the standard BlackBox format.

Thus, if you pass conv = NIL, then no conversion should occur. So, if you try to open a .png file with Views.Old passing conv = NIL, you should receive a NIL, because there is no Documents.Document in a png file, and thus no view can be extracted from a png file with the standard document converter.

However de-facto the implementation (in StdDialog.OldView) calls Converters.Import, passing conv to it witout change. Well,
PROCEDURE Converters.Import (loc: Files.Locator; name: Files.Name; VAR conv: Converter; OUT s: Stores.Store) wrote:If conv = NIL, the first suitable converter in list is used and returned in the VAR parameter.


Thus, Views.Old, upon receiving conv = NIL, looks for the first fitting converters and uses it to read the file. And returns a mess of characters, because png cannot be converted into a View.

How to fix:
It seems to me that something was left in confusion in StdDialog.OldView: it declares a local c: Converters.Converter, performs some manipulations with it and then never uses the result. The local c needs to be removed, and formal conv used everywhere in it's place:
Code: Select all
      c := conv;  (* remove *)
      IF c = NIL THEN c := Converters.list END;   (* use document converter *)
         (* IF conv = NIL THEN conv := Converters.list END; *)
      w := Windows.GetBySpec(loc, name, c (* -> conv *), {});
      IF w # NIL THEN s := w.doc.ThisView() END;
      IF s = NIL THEN
         Converters.Import(loc, name, conv, s);


I move to fix it.

Besides, the docu for Views.Old needs to be fixed, too, IMHO. There needs to be a mention of the case when conversion is impossible, and NIL is returned in that case. The postcondition
result = NIL
loc.res # 0
seems to be inaccurate.

Even under current implementation, a NIL may be returned if no fitting converter is found. This is low chance, since the default Blackbox has a number of 'catch all' converteres installed by default. It is one of these converters that produces the character mess - pretty useless, I would turn off their Converters.importAll flag. However, if they are not installed (in Config), a NIL may well be returned by Views.Old.
User avatar
adimetrius
 
Posts: 34
Joined: Sun Aug 04, 2019 1:02 pm

Re: Views.Old doesn't impement documentation specs

Postby luowy » Sat Jul 11, 2020 1:27 am

Do you have an example to demonstrate your explanation?
luowy
 
Posts: 72
Joined: Thu Dec 17, 2015 1:32 pm

Re: Views.Old doesn't impement documentation specs

Postby adimetrius » Mon Jul 13, 2020 5:44 pm

Iuowy,

I hope this explanation helps:
Code: Select all
conv := NIL;
name := 'picture.png';
loc := Files.dir.This();
v := Views.Old(Views.dontAsk, loc, name, conv);
IF v = NIL THEN Log.String("I expect v = NIL here, because picture.png does not contain (any) View, and I passed NIL for conveter, which means, 'Dont do any conversion'")
ELSE
  WITH v: TextViews.View DO ...
  ELSE
  END
END;
...
Converters.Import(loc, name, conv, v);
(* Here, you can almost certainly assert that v # NIL, because conv = NIL means 'find the best suitable converter', and there usually is a default converter that can at least represent whatever file as text or hex *)
IF v = NIL THEN
ELSE
  WITH v: TextViews.View DO (* I expect v # NIL, although it may contain gibberish text *)
  ELSE
  END
END
User avatar
adimetrius
 
Posts: 34
Joined: Sun Aug 04, 2019 1:02 pm

Re: Views.Old doesn't impement documentation specs

Postby luowy » Tue Jul 14, 2020 7:17 am

after play your code,

Code: Select all
MODULE ObxOldView;

   IMPORT Converters, Views, TextViews, Log := StdLog, Files;

   PROCEDURE OldView* ();
      VAR conv: Converters.Converter;
      name: ARRAY 256 OF CHAR;
      loc: Files.Locator; v: Views.View;
      
   BEGIN
      name :=(* 'picture';*)'picture.png';
      loc := Files.dir.This(""); conv := NIL;
      v := Views.Old(Views.dontAsk, loc, name, conv);
      IF v = NIL THEN
         Log.String("I expect v = NIL here"); Log.Ln;
           (* because picture.png does not contain (any) View, and I passed NIL for conveter,
               which means, 'Dont do any conversion'"*)
      ELSE
         WITH v: TextViews.View DO
            Log.String("v is a TextViews.View"); Log.Ln;
            Log.String("Converter is "+conv.imp);Log.Ln;
            Views.OpenView(v);
         ELSE
            Log.String("v is a Views.View"); Log.Ln;
            Log.String("Converter is "+conv.imp);Log.Ln;
            Views.OpenView(v);
         END
      END;

   END OldView;
   
   PROCEDURE Import* ();
      VAR conv: Converters.Converter;
      name: ARRAY 256 OF CHAR;
      loc: Files.Locator; v: Views.View;
   BEGIN
      name := (*'picture';*)'picture.png';
      loc := Files.dir.This(""); conv := NIL;
      Converters.Import(loc, name, conv, v);

      (* Here, you can almost certainly assert that v # NIL,
         because conv = NIL means 'find the best suitable converter',
         and there usually is a default converter that can at least represent whatever file as text or hex
      *)

      IF v = NIL THEN
      ELSE
         WITH v: TextViews.View DO (* I expect v # NIL, although it may contain gibberish text *)
            Log.String("v is a TextViews.View"); Log.Ln;
            Log.String("Converter is "+conv.imp);Log.Ln;
            Views.OpenView(v);
         ELSE
            Log.String("v is a Views.View"); Log.Ln;
            Log.String("Converter is "+conv.imp);Log.Ln;
            Views.OpenView(v);
         END
      END
   
   END Import;


END ObxOldView.


I see what you mean:
you expect Old return "v=NIL", when input a "alien" file with "conv = NIL", but the framework does not;
and expect Import return "v=NIL", when input a "alien" file with "conv = NIL", framework does same;

IMO, amendment no needed, please check StdApi.OpenAux to understand how to use this procedure;

P.S. loc.res = 77 mean "openFromDisk"
luowy
 
Posts: 72
Joined: Thu Dec 17, 2015 1:32 pm

Re: Views.Old doesn't impement documentation specs

Postby adimetrius » Tue Jul 14, 2020 10:57 am

luowy wrote:I see what you mean:
you expect Old return "v=NIL", when input a "alien" file with "conv = NIL", but the framework does not;
and expect Import return "v=NIL", when input a "alien" file with "conv = NIL", framework does same;


Not exactly:
1) I expect Import to return "v = NIL" when input a "alian" file with "conv = NIL" ONLY if there are no suitable converters installed; the framework does NOT do the same.
2) I expect Import to return "v # NIL" when input a "alian" file with "conv = NIL" if a suitable (possibly a catch-all) converter is installed.

luowy wrote:please check StdApi.OpenAux to understand how to use this procedure;

I checked it, and I cannot see what you're hinting at; can you be more specific?

StdApi.OpenDoc/OpenAux gives wrong error reporting: it will report a file as nonexistent when in fact the file exists but there's no suitable converter for it. Here's how to check: make sure no converters are installed in Config; then
Code: Select all
MODULE VarOpener;
   
   IMPORT Views, StdApi, Log;
   
   PROCEDURE OpenDoc* (IN file: ARRAY OF CHAR);
      VAR v: Views.View;
   BEGIN
      StdApi.OpenDoc(file, v);
      IF v = NIL THEN Log.String("v = NIL"); Log.Ln
      ELSE Views.OpenView(v)
      END
   END OpenDoc;
   
END VarOpener.

Now (!)"VarOpener.OpenDoc('/tmp/3/picture.png')" logs
Log wrote:no converter found
file /tmp/3/picture.png not found

This is WRONG, the file /tmp/3/picture.png is found and doing allright.

In fact, StdApi.OpenDoc supports my second point: the docu for Views.Old is not exact and needs to be amended. Views.Old does return NIL if conversion is impossible. So I maintain my second point: the docu for Views.Old is not exact and needs to be amended.

I still maintain my first point as well: Views.Old implementation should comply with it's documentation and, when passed conv = NIL (and ask = Views.dontAsk), only read from standard BlackBox file format thru the standard document converter. This keeps things simple indeed: if you are only interested in standard BB views and documents, which is the most frequent case IMHO, then you use Views.Old; otherwise, you dig deeper and use Converters.Import.
User avatar
adimetrius
 
Posts: 34
Joined: Sun Aug 04, 2019 1:02 pm

Re: Views.Old doesn't impement documentation specs

Postby luowy » Wed Jul 15, 2020 3:14 am

Usually the documentation is outdated than the source code;
If the documentation is incorrect and no bugs in the source code, it is better to modify the documentation rather than the source code.

This is a basic interface, many modules depend on it, changing it without checking other modules will cause a lot of trouble, and how about third-party modules?

If you are really not satisfied with this API, it is recommended to add a new function, which does no harm to the framework
luowy
 
Posts: 72
Joined: Thu Dec 17, 2015 1:32 pm


Return to BlackBox Framework

Who is online

Users browsing this forum: No registered users and 2 guests

cron