[v5,1/2] Fix output word

Message ID 20180323114224.20407-2-lvivier@redhat.com
State Superseded
Headers show
Series
  • Fix output word
Related show

Commit Message

Laurent Vivier March 23, 2018, 11:42 a.m.
We can select the console output, but it does not really work

Implement term-io-emit, as we have term-io-key to really
send characters to the output selected by stdout.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 slof/fs/term-io.fs | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

Comments

Thomas Huth March 26, 2018, 11:06 a.m. | #1
On 23.03.2018 12:42, Laurent Vivier wrote:
> We can select the console output, but it does not really work
> 
> Implement term-io-emit, as we have term-io-key to really
> send characters to the output selected by stdout.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  slof/fs/term-io.fs | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs
> index 52ce12a..97d02a4 100644
> --- a/slof/fs/term-io.fs
> +++ b/slof/fs/term-io.fs
> @@ -10,6 +10,17 @@
>  \ *     IBM Corporation - initial implementation
>  \ ****************************************************************************/
>  
> +0 VALUE write-xt
> +
> +VARIABLE stdout
> +
> +: set-stdout ( ihandle -- )
> +   \ Close old stdout:
> +   stdout @ ?dup IF close-dev THEN
> +   \ Now set the new stdout:
> +   dup stdout !
> +   encode-int s" stdout" set-chosen
> +;
>  
>  : input  ( dev-str dev-len -- )
>     open-dev ?dup IF
> @@ -24,12 +35,15 @@
>  
>  : output  ( dev-str dev-len -- )
>     open-dev ?dup IF
> -      \ Close old stdout:
> -      s" stdout" get-chosen IF
> -         decode-int nip nip ?dup IF close-dev THEN
> +      \ find new ihandle and xt handle
> +      dup s" write" rot ihandle>phandle find-method
> +      0= IF
> +         drop
> +         cr ." Cannot find the write method for the given output console " cr
> +         EXIT
>        THEN
> -      \ Now set the new stdout:
> -      encode-int s" stdout" set-chosen
> +      to write-xt
> +      set-stdout
>     THEN
>  ;
>  
> @@ -40,6 +54,15 @@
>  
>  1 BUFFER: (term-io-char-buf)
>  
> +: term-io-emit ( char -- )
> +    write-xt 0= IF drop EXIT THEN

Patch looks basically fine to me, but this part is a little bit ugly: As
long as write-xt is 0, there won't be a way to print out any console
output. So between the point in time where term-io.fs is included (i.e.
tree.fs in OF.fs) and the point in time where "output" is called, there
won't be any console output anymore.
To fix this, use the trick that is already used in term-io-key:

  write-xt 0= IF [ ' emit behavior compile, ] THEN

> +    (term-io-char-buf) c!
> +    (term-io-char-buf) 1 write-xt stdout @ call-package
> +    drop
> +;
> +
> +' term-io-emit to emit

 Thomas
Thomas Huth March 26, 2018, 11:11 a.m. | #2
On 26.03.2018 13:06, Thomas Huth wrote:
> On 23.03.2018 12:42, Laurent Vivier wrote:
>> We can select the console output, but it does not really work
>>
>> Implement term-io-emit, as we have term-io-key to really
>> send characters to the output selected by stdout.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  slof/fs/term-io.fs | 33 ++++++++++++++++++++++++++++-----
>>  1 file changed, 28 insertions(+), 5 deletions(-)
>>
>> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs
>> index 52ce12a..97d02a4 100644
>> --- a/slof/fs/term-io.fs
>> +++ b/slof/fs/term-io.fs
>> @@ -10,6 +10,17 @@
>>  \ *     IBM Corporation - initial implementation
>>  \ ****************************************************************************/
>>  
>> +0 VALUE write-xt
>> +
>> +VARIABLE stdout
>> +
>> +: set-stdout ( ihandle -- )
>> +   \ Close old stdout:
>> +   stdout @ ?dup IF close-dev THEN
>> +   \ Now set the new stdout:
>> +   dup stdout !
>> +   encode-int s" stdout" set-chosen
>> +;
>>  
>>  : input  ( dev-str dev-len -- )
>>     open-dev ?dup IF
>> @@ -24,12 +35,15 @@
>>  
>>  : output  ( dev-str dev-len -- )
>>     open-dev ?dup IF
>> -      \ Close old stdout:
>> -      s" stdout" get-chosen IF
>> -         decode-int nip nip ?dup IF close-dev THEN
>> +      \ find new ihandle and xt handle
>> +      dup s" write" rot ihandle>phandle find-method
>> +      0= IF
>> +         drop
>> +         cr ." Cannot find the write method for the given output console " cr
>> +         EXIT
>>        THEN
>> -      \ Now set the new stdout:
>> -      encode-int s" stdout" set-chosen
>> +      to write-xt
>> +      set-stdout
>>     THEN
>>  ;
>>  
>> @@ -40,6 +54,15 @@
>>  
>>  1 BUFFER: (term-io-char-buf)
>>  
>> +: term-io-emit ( char -- )
>> +    write-xt 0= IF drop EXIT THEN
> 
> Patch looks basically fine to me, but this part is a little bit ugly: As
> long as write-xt is 0, there won't be a way to print out any console
> output. So between the point in time where term-io.fs is included (i.e.
> tree.fs in OF.fs) and the point in time where "output" is called, there
> won't be any console output anymore.
> To fix this, use the trick that is already used in term-io-key:
> 
>   write-xt 0= IF [ ' emit behavior compile, ] THEN

And of course there should also be an "EXIT" before the "THEN" in this
case :-)

 Thomas
Laurent Vivier March 27, 2018, 11:23 a.m. | #3
On 26/03/2018 13:11, Thomas Huth wrote:
> On 26.03.2018 13:06, Thomas Huth wrote:
>> On 23.03.2018 12:42, Laurent Vivier wrote:
>>> We can select the console output, but it does not really work
>>>
>>> Implement term-io-emit, as we have term-io-key to really
>>> send characters to the output selected by stdout.
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>  slof/fs/term-io.fs | 33 ++++++++++++++++++++++++++++-----
>>>  1 file changed, 28 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs
>>> index 52ce12a..97d02a4 100644
>>> --- a/slof/fs/term-io.fs
>>> +++ b/slof/fs/term-io.fs
>>> @@ -10,6 +10,17 @@
>>>  \ *     IBM Corporation - initial implementation
>>>  \ ****************************************************************************/
>>>  
>>> +0 VALUE write-xt
>>> +
>>> +VARIABLE stdout
>>> +
>>> +: set-stdout ( ihandle -- )
>>> +   \ Close old stdout:
>>> +   stdout @ ?dup IF close-dev THEN
>>> +   \ Now set the new stdout:
>>> +   dup stdout !
>>> +   encode-int s" stdout" set-chosen
>>> +;
>>>  
>>>  : input  ( dev-str dev-len -- )
>>>     open-dev ?dup IF
>>> @@ -24,12 +35,15 @@
>>>  
>>>  : output  ( dev-str dev-len -- )
>>>     open-dev ?dup IF
>>> -      \ Close old stdout:
>>> -      s" stdout" get-chosen IF
>>> -         decode-int nip nip ?dup IF close-dev THEN
>>> +      \ find new ihandle and xt handle
>>> +      dup s" write" rot ihandle>phandle find-method
>>> +      0= IF
>>> +         drop
>>> +         cr ." Cannot find the write method for the given output console " cr
>>> +         EXIT
>>>        THEN
>>> -      \ Now set the new stdout:
>>> -      encode-int s" stdout" set-chosen
>>> +      to write-xt
>>> +      set-stdout
>>>     THEN
>>>  ;
>>>  
>>> @@ -40,6 +54,15 @@
>>>  
>>>  1 BUFFER: (term-io-char-buf)
>>>  
>>> +: term-io-emit ( char -- )
>>> +    write-xt 0= IF drop EXIT THEN
>>
>> Patch looks basically fine to me, but this part is a little bit ugly: As
>> long as write-xt is 0, there won't be a way to print out any console
>> output. So between the point in time where term-io.fs is included (i.e.
>> tree.fs in OF.fs) and the point in time where "output" is called, there
>> won't be any console output anymore.
>> To fix this, use the trick that is already used in term-io-key:
>>
>>   write-xt 0= IF [ ' emit behavior compile, ] THEN
> 
> And of course there should also be an "EXIT" before the "THEN" in this
> case :-)

I did this in v2 of the patch, but Segher said:

> That is very problematic.  If the xt of term-io-emit already is the
> behaviour of emit, this will loop.  It sounds like you just want the
> "system emit"; just assign that to the hook, don't do extra redirections?

But I don't know to have only "system emit".

Any advice?

Thanks,
Laurent
Thomas Huth March 27, 2018, 12:03 p.m. | #4
On 27.03.2018 13:23, Laurent Vivier wrote:
> On 26/03/2018 13:11, Thomas Huth wrote:
>> On 26.03.2018 13:06, Thomas Huth wrote:
>>> On 23.03.2018 12:42, Laurent Vivier wrote:
>>>> We can select the console output, but it does not really work
>>>>
>>>> Implement term-io-emit, as we have term-io-key to really
>>>> send characters to the output selected by stdout.
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>>  slof/fs/term-io.fs | 33 ++++++++++++++++++++++++++++-----
>>>>  1 file changed, 28 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs
>>>> index 52ce12a..97d02a4 100644
>>>> --- a/slof/fs/term-io.fs
>>>> +++ b/slof/fs/term-io.fs
>>>> @@ -10,6 +10,17 @@
>>>>  \ *     IBM Corporation - initial implementation
>>>>  \ ****************************************************************************/
>>>>  
>>>> +0 VALUE write-xt
>>>> +
>>>> +VARIABLE stdout
>>>> +
>>>> +: set-stdout ( ihandle -- )
>>>> +   \ Close old stdout:
>>>> +   stdout @ ?dup IF close-dev THEN
>>>> +   \ Now set the new stdout:
>>>> +   dup stdout !
>>>> +   encode-int s" stdout" set-chosen
>>>> +;
>>>>  
>>>>  : input  ( dev-str dev-len -- )
>>>>     open-dev ?dup IF
>>>> @@ -24,12 +35,15 @@
>>>>  
>>>>  : output  ( dev-str dev-len -- )
>>>>     open-dev ?dup IF
>>>> -      \ Close old stdout:
>>>> -      s" stdout" get-chosen IF
>>>> -         decode-int nip nip ?dup IF close-dev THEN
>>>> +      \ find new ihandle and xt handle
>>>> +      dup s" write" rot ihandle>phandle find-method
>>>> +      0= IF
>>>> +         drop
>>>> +         cr ." Cannot find the write method for the given output console " cr
>>>> +         EXIT
>>>>        THEN
>>>> -      \ Now set the new stdout:
>>>> -      encode-int s" stdout" set-chosen
>>>> +      to write-xt
>>>> +      set-stdout
>>>>     THEN
>>>>  ;
>>>>  
>>>> @@ -40,6 +54,15 @@
>>>>  
>>>>  1 BUFFER: (term-io-char-buf)
>>>>  
>>>> +: term-io-emit ( char -- )
>>>> +    write-xt 0= IF drop EXIT THEN
>>>
>>> Patch looks basically fine to me, but this part is a little bit ugly: As
>>> long as write-xt is 0, there won't be a way to print out any console
>>> output. So between the point in time where term-io.fs is included (i.e.
>>> tree.fs in OF.fs) and the point in time where "output" is called, there
>>> won't be any console output anymore.
>>> To fix this, use the trick that is already used in term-io-key:
>>>
>>>   write-xt 0= IF [ ' emit behavior compile, ] THEN
>>
>> And of course there should also be an "EXIT" before the "THEN" in this
>> case :-)
> 
> I did this in v2 of the patch, but Segher said:
> 
>> That is very problematic.  If the xt of term-io-emit already is the
>> behaviour of emit, this will loop.

I'm afraid to object Segher, but as far as I can see, this can not
happen. The "[ ' emit behavior compile, ]" happens *while* the Forth
word is compiled, and the first "' term-io-emit to emit" happens *after*
the Forth word is compiled. So how should the term-io-emit XT end up in
"emit" before the term-io-emit Forth word is compiled?

Since we're also using that logic in term-io-key and never encountered a
loop here so far, I think it's safe to go ahead and use this for
term-io-emit, too.

 Thomas
Segher Boessenkool March 27, 2018, 5:54 p.m. | #5
On Tue, Mar 27, 2018 at 02:03:41PM +0200, Thomas Huth wrote:
> >> That is very problematic.  If the xt of term-io-emit already is the
> >> behaviour of emit, this will loop.
> 
> I'm afraid to object Segher, but as far as I can see, this can not
> happen. The "[ ' emit behavior compile, ]" happens *while* the Forth
> word is compiled, and the first "' term-io-emit to emit" happens *after*
> the Forth word is compiled. So how should the term-io-emit XT end up in
> "emit" before the term-io-emit Forth word is compiled?

Exactly as you say for example: if it was done *before*.

This [ ... behavior ... ] stuff is very fragile.  It does not have
to be that way.

Why don't you set debug versions to the emit hook very early?  Why
won't that work?


Segher
Laurent Vivier March 27, 2018, 6:48 p.m. | #6
On 27/03/2018 19:54, Segher Boessenkool wrote:
> On Tue, Mar 27, 2018 at 02:03:41PM +0200, Thomas Huth wrote:
>>>> That is very problematic.  If the xt of term-io-emit already is the
>>>> behaviour of emit, this will loop.
>>
>> I'm afraid to object Segher, but as far as I can see, this can not
>> happen. The "[ ' emit behavior compile, ]" happens *while* the Forth
>> word is compiled, and the first "' term-io-emit to emit" happens *after*
>> the Forth word is compiled. So how should the term-io-emit XT end up in
>> "emit" before the term-io-emit Forth word is compiled?
> 
> Exactly as you say for example: if it was done *before*.
> 
> This [ ... behavior ... ] stuff is very fragile.  It does not have
> to be that way.
> 
> Why don't you set debug versions to the emit hook very early?  Why
> won't that work?

My FORTH knowledge is not good enough to know how to do that...

Could you show me how to do that?

Thanks,
Laurent
Segher Boessenkool March 27, 2018, 10:41 p.m. | #7
On Tue, Mar 27, 2018 at 08:48:35PM +0200, Laurent Vivier wrote:
> On 27/03/2018 19:54, Segher Boessenkool wrote:
> > On Tue, Mar 27, 2018 at 02:03:41PM +0200, Thomas Huth wrote:
> >>>> That is very problematic.  If the xt of term-io-emit already is the
> >>>> behaviour of emit, this will loop.
> >>
> >> I'm afraid to object Segher, but as far as I can see, this can not
> >> happen. The "[ ' emit behavior compile, ]" happens *while* the Forth
> >> word is compiled, and the first "' term-io-emit to emit" happens *after*
> >> the Forth word is compiled. So how should the term-io-emit XT end up in
> >> "emit" before the term-io-emit Forth word is compiled?
> > 
> > Exactly as you say for example: if it was done *before*.
> > 
> > This [ ... behavior ... ] stuff is very fragile.  It does not have
> > to be that way.
> > 
> > Why don't you set debug versions to the emit hook very early?  Why
> > won't that work?
> 
> My FORTH knowledge is not good enough to know how to do that...
> 
> Could you show me how to do that?

Just

' early-debug-emit IS emit

or similar.  If you get problems of not having defined things where you
want to first refer to them, you can always add a second hook (assign
one to the other, the same way you use with DEFER for other forward
declarations).  It is better to define things in a better order, of
course, but forward refs are handy sometimes.


Segher

Patch

diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs
index 52ce12a..97d02a4 100644
--- a/slof/fs/term-io.fs
+++ b/slof/fs/term-io.fs
@@ -10,6 +10,17 @@ 
 \ *     IBM Corporation - initial implementation
 \ ****************************************************************************/
 
+0 VALUE write-xt
+
+VARIABLE stdout
+
+: set-stdout ( ihandle -- )
+   \ Close old stdout:
+   stdout @ ?dup IF close-dev THEN
+   \ Now set the new stdout:
+   dup stdout !
+   encode-int s" stdout" set-chosen
+;
 
 : input  ( dev-str dev-len -- )
    open-dev ?dup IF
@@ -24,12 +35,15 @@ 
 
 : output  ( dev-str dev-len -- )
    open-dev ?dup IF
-      \ Close old stdout:
-      s" stdout" get-chosen IF
-         decode-int nip nip ?dup IF close-dev THEN
+      \ find new ihandle and xt handle
+      dup s" write" rot ihandle>phandle find-method
+      0= IF
+         drop
+         cr ." Cannot find the write method for the given output console " cr
+         EXIT
       THEN
-      \ Now set the new stdout:
-      encode-int s" stdout" set-chosen
+      to write-xt
+      set-stdout
    THEN
 ;
 
@@ -40,6 +54,15 @@ 
 
 1 BUFFER: (term-io-char-buf)
 
+: term-io-emit ( char -- )
+    write-xt 0= IF drop EXIT THEN
+    (term-io-char-buf) c!
+    (term-io-char-buf) 1 write-xt stdout @ call-package
+    drop
+;
+
+' term-io-emit to emit
+
 : term-io-key  ( -- char )
    s" stdin" get-chosen IF
       decode-int nip nip dup 0= IF 0 EXIT THEN