Bug or not a bug? dot expansion in ob-shell

Bug or not a bug? dot expansion in ob-shell

[Vladimir Nikishkin]

[-- Attachment #1: Type: text/plain, Size: 154 bytes --]

Hello, everyone

Could you check if the following block behaves as expected?

#+begin_src shell
echo .
#+end_src

-- 
Yours sincerely, Vladimir Nikishkin

[-- Attachment #2: Type: text/html, Size: 342 bytes --]

Re: Bug or not a bug? dot expansion in ob-shell

[Fraga, Eric]

On Wednesday, 19 Feb 2020 at 17:02, Vladimir Nikishkin wrote:
> Could you check if the following block behaves as expected?

It does for me.  What did you expect and what did you get?

-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-316-g5dd772

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Eric and Vladimir,

"Fraga, Eric" <e.fraga@ucl.ac.uk> writes:

> On Wednesday, 19 Feb 2020 at 17:02, Vladimir Nikishkin wrote:
>> Could you check if the following block behaves as expected?
>
> It does for me.  What did you expect and what did you get?

It returned "0" for me, while I guess "." is expected.

The function org-babel--string-to-number was not interpreting
Emacs numbers correctly: for example, -1e3 would not return a
number, while it should.

I've now fixed it in maint.

Let me know if it does the right thing for you.

Thanks,

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Bastien <bzg@gnu.org> writes:

> Let me know if it does the right thing for you.

PS: Ouch, I broke a few tests.  I'm looking at this right now.

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Bastien <bzg@gnu.org> writes:

> PS: Ouch, I broke a few tests.  I'm looking at this right now.

Those have been fix, together with org-babel--string-to-number,
which should not allow "1 2" to be interpreted as a number.

Eric, if my reading of the expected output does not match yours
please let me know.

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Fraga, Eric]

On Wednesday, 19 Feb 2020 at 10:41, Bastien wrote:
> It returned "0" for me, while I guess "." is expected.

I expected 0 as that is the "value" (i.e. the status in a shell) of the
last command.  If you want ".", I would expect one to use :results
output in the src block header?

-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-316-g5dd772

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Eric,

"Fraga, Eric" <e.fraga@ucl.ac.uk> writes:

> On Wednesday, 19 Feb 2020 at 10:41, Bastien wrote:
>> It returned "0" for me, while I guess "." is expected.
>
> I expected 0 as that is the "value" (i.e. the status in a shell) of
> the last command.

Quoting the manual:

‘value’
     Default.  Functional mode.  Org gets the value by wrapping the code
     in a function definition in the language of the source block.  That
     is why when using ‘:results value’, code should execute like a
     function and return a value.  For languages like Python, an
     explicit ‘return’ statement is mandatory when using ‘:results
     value’.  Result is the value returned by the last statement in the
     code block.

"0" is the _exit code_ of the successful echo command, not the value
returned by the echo command.

So In Vladimir's example, both ":results value" and ":results output"
should return the same result, i.e. ".".

Was it common to expect the exit code when executing shell code?

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Fraga, Eric]

On Wednesday, 19 Feb 2020 at 12:38, Bastien wrote:
> "0" is the _exit code_ of the successful echo command, not the value
> returned by the echo command.

But echo does not "return" the string as a value.  It outputs the
string.

To quote the man page for bash, "the return value of a simple command is
its status".  Further, a function does not actually return any value
beyond the status of the last command or a value given on a =return=
statement.

> So In Vladimir's example, both ":results value" and ":results output"
> should return the same result, i.e. ".".

I disagree.  I think the current behaviour (i.e. before your attempt to
"correct"" this) is correct given the documentation you quoted!

> Was it common to expect the exit code when executing shell code?

Common?  I have no idea.  *I* did expect this.  But that's maybe because
I do use the shell a lot.

I think there's a clear distinction between value and output for src
blocks and blurring this distinction for shell src blocks would be
misleading.  The option to request the output as the outcome of the src
block is already there.
-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-345-g415083

Re: Bug or not a bug? dot expansion in ob-shell

[Vladimir Nikishkin]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

That's an excellent response, Eric!

Now what about this block:

#+begin_src shell
  printf "%s" '
  (computer . ?type)
  exit'
#+end_src

ср, 19 февр. 2020 г. в 19:56, Fraga, Eric <e.fraga@ucl.ac.uk>:

> On Wednesday, 19 Feb 2020 at 12:38, Bastien wrote:
> > "0" is the _exit code_ of the successful echo command, not the value
> > returned by the echo command.
>
> But echo does not "return" the string as a value.  It outputs the
> string.
>
> To quote the man page for bash, "the return value of a simple command is
> its status".  Further, a function does not actually return any value
> beyond the status of the last command or a value given on a =return=
> statement.
>
> > So In Vladimir's example, both ":results value" and ":results output"
> > should return the same result, i.e. ".".
>
> I disagree.  I think the current behaviour (i.e. before your attempt to
> "correct"" this) is correct given the documentation you quoted!
>
> > Was it common to expect the exit code when executing shell code?
>
> Common?  I have no idea.  *I* did expect this.  But that's maybe because
> I do use the shell a lot.
>
> I think there's a clear distinction between value and output for src
> blocks and blurring this distinction for shell src blocks would be
> misleading.  The option to request the output as the outcome of the src
> block is already there.
> --
> : Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-345-g415083
>


-- 
Yours sincerely, Vladimir Nikishkin

[-- Attachment #2: Type: text/html, Size: 2131 bytes --]

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Eric,

note that the previous behavior only _seemed_ right by chance: there
is no notion of getting the exit code of the shell command in
ob-shell.el, and returning "0" is just a hazard here, just because
(org-babel--string-to-number ".") returns "0", while it should return
nil.

"Fraga, Eric" <e.fraga@ucl.ac.uk> writes:

> On Wednesday, 19 Feb 2020 at 12:38, Bastien wrote:
>> "0" is the _exit code_ of the successful echo command, not the value
>> returned by the echo command.
>
> But echo does not "return" the string as a value.  It outputs the
> string.
>
> To quote the man page for bash, "the return value of a simple command is
> its status".  Further, a function does not actually return any value
> beyond the status of the last command or a value given on a =return=
> statement.

Then we need to fix ob-shell.el to return the exit code of the last
command when :results is not set or explicitely set to "value".  Is
this something you want to look at?

Maybe by adding a "echo $?" instruction at the end of shell blocks
or by wrapping the code into something that returns the result?

I think it will come as a surprise for many users, since the natural
expectation seems to get the "output", disregarding bash notion of a
"return value".

I don't know.

> I disagree.  I think the current behaviour (i.e. before your attempt to
> "correct"" this) is correct given the documentation you quoted!

Yes, I see how it seems correct, but this was random...

>> Was it common to expect the exit code when executing shell code?
>
> Common?  I have no idea.  *I* did expect this.  But that's maybe because
> I do use the shell a lot.

I won't release 9.4 until we properly fix this, it's important.

> I think there's a clear distinction between value and output for src
> blocks and blurring this distinction for shell src blocks would be
> misleading.

Agreed.

> The option to request the output as the outcome of the src block is
> already there.

Yes, agreed again.

If you or someone else can look at ob-shell.el and see what can be
done to get the proper value (in bash's terms) of the last command in
the block, that'd be great.

Thanks!

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Bastien <bzg@gnu.org> writes:

> Then we need to fix ob-shell.el to return the exit code of the last
> command when :results is not set or explicitely set to "value".  Is
> this something you want to look at?
>
> Maybe by adding a "echo $?" instruction at the end of shell blocks
> or by wrapping the code into something that returns the result?
>
> I think it will come as a surprise for many users, since the natural
> expectation seems to get the "output", disregarding bash notion of a
> "return value".

Maybe ob-shell.sh could have an option ob-shell-value-is-exit-status
set to nil by default to stick with the current behavior, but letting
users get the exit status as the value if they want to.

WDYT?

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Tim Cross]

I think I agree with Eric here. However, perhaps I don't fully
understand all the details.

Many shell commans, especially those
which mainly perform 'side effects' like echo, return the exit code.
This value is often used in shell scripts as a type of short hand/short
circuit i.e. combined with logic operators.

Question: would it not be import to separate return value from output
value when it comes to shell scripts and noweb type expansion e.g. if I
had a block where the last command does not do any output, but does
return an exit value as the return value, might it not be important to
have that exit value for the next babel block which uses the previous
block?

Something else which may be relevant is to note that bash is not the
only shell and some commands, like echo, will behave differently
depending on the shell. For exmaple, in some shells, echo with no
arguments will just ouput a newline while on other shells, you need to
explicitly request this behaviour with -n switch.

As a general principal, I think the return value and the output value
should be considered to be distinct items and it would be a mistake to
return the output value when it appears the last command does not return
a value. Do what the shell would do - if it returns the exit code then
return that. If the last command really doesn't return anything i.e. hot
even an exit code, return nil (though I think under posix, at the very
least, the exit code is returned).

Fraga, Eric <e.fraga@ucl.ac.uk> writes:

> On Wednesday, 19 Feb 2020 at 12:38, Bastien wrote:
>> "0" is the _exit code_ of the successful echo command, not the value
>> returned by the echo command.
>
> But echo does not "return" the string as a value.  It outputs the
> string.
>
> To quote the man page for bash, "the return value of a simple command is
> its status".  Further, a function does not actually return any value
> beyond the status of the last command or a value given on a =return=
> statement.
>
>> So In Vladimir's example, both ":results value" and ":results output"
>> should return the same result, i.e. ".".
>
> I disagree.  I think the current behaviour (i.e. before your attempt to
> "correct"" this) is correct given the documentation you quoted!
>
>> Was it common to expect the exit code when executing shell code?
>
> Common?  I have no idea.  *I* did expect this.  But that's maybe because
> I do use the shell a lot.
>
> I think there's a clear distinction between value and output for src
> blocks and blurring this distinction for shell src blocks would be
> misleading.  The option to request the output as the outcome of the src
> block is already there.


--
Tim Cross

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Tim,

thanks for your proposal.  I think we agree here.

My suggestion is to have a new option ob-shell-value-is-exit-status.

When set to nil (the default), the "return value" of a shell source
block would be the output of the last command.  This is the current
behavior where we have e.g.

  #+begin_src shell
  echo "Hello!"
  #+end_src
  
  #+RESULTS:
  : Hello!

When set to t, the return value of a shell source block would be the
exit code of the last command.  This would be useful for side effect
and other use cases.

We can also consider a specific parameter :value-is-exit-code to set
for individual blocks--useful for noweb.

My only point is I don't think ob-shell-value-is-exit-status should be
t by default, as it would cause all shell blocks to return the status
code by default, which may not be what most users want.

Anyway, I don't have yet a clue on how to add this new option.  I'll
leave it to Eric first (if he has time) then look at it later this
week.

Thanks!

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Tim Cross]

Hi Bastien,

I'm not sure - need to think about it some more and go back through the
manual. My initial feeling is that the example block you show should
only return "Hello!" when you request output as the reuslts and
otherwise return the return value of echo (which is the exit code in
this case).

The main problem with the additional variable you propose is that you
would only want it enabled on some source blocks and not others, so it
might need to be settable as a header option to turn it on/off for a
specific block.

Bastien <bzg@gnu.org> writes:

> Hi Tim,
>
> thanks for your proposal.  I think we agree here.
>
> My suggestion is to have a new option ob-shell-value-is-exit-status.
>
> When set to nil (the default), the "return value" of a shell source
> block would be the output of the last command.  This is the current
> behavior where we have e.g.
>
>   #+begin_src shell
>   echo "Hello!"
>   #+end_src
>
>   #+RESULTS:
>   : Hello!
>
> When set to t, the return value of a shell source block would be the
> exit code of the last command.  This would be useful for side effect
> and other use cases.
>
> We can also consider a specific parameter :value-is-exit-code to set
> for individual blocks--useful for noweb.
>
> My only point is I don't think ob-shell-value-is-exit-status should be
> t by default, as it would cause all shell blocks to return the status
> code by default, which may not be what most users want.
>
> Anyway, I don't have yet a clue on how to add this new option.  I'll
> leave it to Eric first (if he has time) then look at it later this
> week.
>
> Thanks!


--
Tim Cross

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Tim,

Tim Cross <theophilusx@gmail.com> writes:

> I'm not sure - need to think about it some more and go back through the
> manual. My initial feeling is that the example block you show should
> only return "Hello!" when you request output as the reuslts and
> otherwise return the return value of echo (which is the exit code in
> this case).

Yes - my point in this thread is that for now it returns "Hello!" and
I guess this is a natural thing to expect.

> The main problem with the additional variable you propose is that you
> would only want it enabled on some source blocks and not others, so it
> might need to be settable as a header option to turn it on/off for a
> specific block.

Yes, hence the proposal to have :value-is-exit-code as a header arg.

Let's find the right approach to this.

Thanks,

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Fraga, Eric]

On Wednesday, 19 Feb 2020 at 14:00, Bastien wrote:
> Anyway, I don't have yet a clue on how to add this new option.  I'll
> leave it to Eric first (if he has time) then look at it later this
> week.

Time is an issue (middle of teaching term).  More importantly, my
elisp-fu is not necessarily up to the level required.  It would be best
if somebody up to speed with org babel attempt this.

In any case, it would be interesting to know if people do depend on
value being somehow the output of the last command in a shell
script.  I'd never even considered that possibility, at least not
consciously!

-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-345-g415083

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Eric,

"Fraga, Eric" <e.fraga@ucl.ac.uk> writes:

> On Wednesday, 19 Feb 2020 at 14:00, Bastien wrote:
>> Anyway, I don't have yet a clue on how to add this new option.  I'll
>> leave it to Eric first (if he has time) then look at it later this
>> week.
>
> Time is an issue (middle of teaching term).  More importantly, my
> elisp-fu is not necessarily up to the level required.  It would be best
> if somebody up to speed with org babel attempt this.

I will have a look.

> In any case, it would be interesting to know if people do depend on
> value being somehow the output of the last command in a shell
> script.  I'd never even considered that possibility, at least not
> consciously!

Note that currently (9.3) we have this: 

#+begin_src shell
echo "Hello!"
echo "What?"
#+end_src

#+RESULTS:
| Hello! |
| What?  |

... so the default today is not even to consider "the last command",
but the output of all commands.

Have a nice week,

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Fraga, Eric]

On Wednesday, 19 Feb 2020 at 14:43, Bastien wrote:
> Note that currently (9.3) we have this: 

[...]

> ... so the default today is not even to consider "the last command",
> but the output of all commands.

Maybe, if you wish to get version 9.4 out, the best approach would be to
fix that one outlier case ("." giving "0") for now and later handle
value/output distinction properly (should it be desired, of course)?

-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-350-g39f1c1

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

I've implemented the suggestion I made in master:

  New option ~ob-shell-return-value-is-exit-status~
  
  When set to =t=, consider the return value of a shell source code
  block is the exit status of its last command.  
  
  The default for this option is =nil=, i.e. the return value of a shell
  block is the output of the commands.
  
  You can turn this on individual blocks by setting the header argument
  =:value-is-exit-status= to =t=.

Please test and let me know if it works.

Thanks for bringing this up!

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Diego Zamboni]

[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]

I'm late to the discussion so I apologize in advance, but this fix seems
counterintuitive to me. In my mind, for any shell code:

- Return value: exit code of the last command
- Output: whatever the commands print

So to me, it's intuitive that =:exports value= would return the exit code
of the last command, and =:exports output= would produce the output of the
commands. I don't understand why a new option is needed.

Am I missing something?

--Diego


On Wed, Feb 19, 2020 at 5:00 PM Bastien <bzg@gnu.org> wrote:

> I've implemented the suggestion I made in master:
>
>   New option ~ob-shell-return-value-is-exit-status~
>
>   When set to =t=, consider the return value of a shell source code
>   block is the exit status of its last command.
>
>   The default for this option is =nil=, i.e. the return value of a shell
>   block is the output of the commands.
>
>   You can turn this on individual blocks by setting the header argument
>   =:value-is-exit-status= to =t=.
>
> Please test and let me know if it works.
>
> Thanks for bringing this up!
>
> --
>  Bastien
>
>

[-- Attachment #2: Type: text/html, Size: 1566 bytes --]

Re: Bug or not a bug? dot expansion in ob-shell

[Samuel Wales]

to stir the pot:

speaking for myself, i only wanted the output, and never the error
mechanism, and my attempts to get output reliably ended up in my
habitually putting in every shell block this boilerplate:

===
{
the code i actually want
} 2>&1
:
===

-- 
The Kafka Pandemic

What is misopathy?
https://thekafkapandemic.blogspot.com/2013/10/why-some-diseases-are-wronged.html

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Diego,

Diego Zamboni <diego@zzamboni.org> writes:

> I'm late to the discussion so I apologize in advance, but this fix
> seems counterintuitive to me. In my mind, for any shell code:
>
> - Return value: exit code of the last command
> - Output: whatever the commands print

Yes, that's what is *now* possible if you set
ob-shell-return-value-is-exit-status to t.

Unless I miss something, it was not possible before today.

#+begin_src shell
echo Hello!
#+end_src

would simply return "Hello!" as a return.

No exit code was *never* output.

> So to me, it's intuitive that =:exports value= would return the exit
> code of the last command, and =:exports output= would produce the
> output of the commands. I don't understand why a new option is
> needed.

... because it was not the case before.  Or maybe *I* miss something.

Can you show me something that was working before and that is not
anymore?

Thanks,

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Nick Dokos]

Hi Bastien,

Bastien <bzg@gnu.org> writes:

> Hi Diego,
>
> Diego Zamboni <diego@zzamboni.org> writes:
>
>> I'm late to the discussion so I apologize in advance, but this fix
>> seems counterintuitive to me. In my mind, for any shell code:
>>
>> - Return value: exit code of the last command
>> - Output: whatever the commands print
>
> Yes, that's what is *now* possible if you set
> ob-shell-return-value-is-exit-status to t.
>
> Unless I miss something, it was not possible before today.
>
> #+begin_src shell
> echo Hello!
> #+end_src
>
> would simply return "Hello!" as a return.
>
> No exit code was *never* output.
>
>> So to me, it's intuitive that =:exports value= would return the exit
>> code of the last command, and =:exports output= would produce the
>> output of the commands. I don't understand why a new option is
>> needed.
>
> ... because it was not the case before.  Or maybe *I* miss something.
>
> Can you show me something that was working before and that is not
> anymore?
>
> Thanks,

I welcome the fix but not the option: the option situation in Org mode
was pretty horrible, then ca 2010, you did a survey and some options
were eliminated (not sure about the year or whether it *was* you who
did the survey, but I'm pretty sure there was one): that was the right
direction to go, but it wasn't enough. Org mode needs to be put into
an option diet, so adding another one here (and a rather gratuitous
one in my view) is not the way to go.

`:results value' should *always* produce the value of the last expression,
which for shell programs is the exit status of the last command.

`:results output' should *always* produce all of the output of the program.

An argument can be made that `:results value' is the default, but it
is the less useful option for shell programs. So maybe for shell
blocks, make the default to be `:results output' instead: people get
what they always got before the fix without lifting a finger, the exit status
is now available with `:results value', and the option can go away
quietly and quickly, before it becomes another contributor to the Org
mode technical debt.

-- 
Nick

"There are only two hard problems in computer science: cache
invalidation, naming things, and off-by-one errors." -Martin Fowler

Re: Bug or not a bug? dot expansion in ob-shell

[Tim Cross]

+1 - this is basically my feeling as well. I've spent the last couple of
days thinking about the additional option suggestion, but something just
didn't feel right to me. I think Nick has hit the nail on the head.

Adding the additional option seems to be making things more confusing.
The basic idea that result value is what the block returns i.e. the
return value of the last statement for shell blocks and result output is
what the code in the block sends to stdout/stderr. This seems the most
intuitive to me.


Nick Dokos <ndokos@gmail.com> writes:

> Hi Bastien,
>
> Bastien <bzg@gnu.org> writes:
>
>> Hi Diego,
>>
>> Diego Zamboni <diego@zzamboni.org> writes:
>>
>>> I'm late to the discussion so I apologize in advance, but this fix
>>> seems counterintuitive to me. In my mind, for any shell code:
>>>
>>> - Return value: exit code of the last command
>>> - Output: whatever the commands print
>>
>> Yes, that's what is *now* possible if you set
>> ob-shell-return-value-is-exit-status to t.
>>
>> Unless I miss something, it was not possible before today.
>>
>> #+begin_src shell
>> echo Hello!
>> #+end_src
>>
>> would simply return "Hello!" as a return.
>>
>> No exit code was *never* output.
>>
>>> So to me, it's intuitive that =:exports value= would return the exit
>>> code of the last command, and =:exports output= would produce the
>>> output of the commands. I don't understand why a new option is
>>> needed.
>>
>> ... because it was not the case before.  Or maybe *I* miss something.
>>
>> Can you show me something that was working before and that is not
>> anymore?
>>
>> Thanks,
>
> I welcome the fix but not the option: the option situation in Org mode
> was pretty horrible, then ca 2010, you did a survey and some options
> were eliminated (not sure about the year or whether it *was* you who
> did the survey, but I'm pretty sure there was one): that was the right
> direction to go, but it wasn't enough. Org mode needs to be put into
> an option diet, so adding another one here (and a rather gratuitous
> one in my view) is not the way to go.
>
> `:results value' should *always* produce the value of the last expression,
> which for shell programs is the exit status of the last command.
>
> `:results output' should *always* produce all of the output of the program.
>
> An argument can be made that `:results value' is the default, but it
> is the less useful option for shell programs. So maybe for shell
> blocks, make the default to be `:results output' instead: people get
> what they always got before the fix without lifting a finger, the exit status
> is now available with `:results value', and the option can go away
> quietly and quickly, before it becomes another contributor to the Org
> mode technical debt.


--
Tim Cross

Re: Bug or not a bug? dot expansion in ob-shell

[Derek Feichtinger]

+1 - I also think that this is the correct behavior, and that the
average user's expectations can be best fulfilled by making ":results output"
the default. Adding the option will make it more difficult to share code
blocks and documents.

Best regards,
Derek

On Thu, Feb 20 2020, Tim Cross <theophilusx@gmail.com> wrote:

> +1 - this is basically my feeling as well. I've spent the last couple of
> days thinking about the additional option suggestion, but something just
> didn't feel right to me. I think Nick has hit the nail on the head.
>
> Adding the additional option seems to be making things more confusing.
> The basic idea that result value is what the block returns i.e. the
> return value of the last statement for shell blocks and result output is
> what the code in the block sends to stdout/stderr. This seems the most
> intuitive to me.
>
>
> Nick Dokos <ndokos@gmail.com> writes:
>
>> Hi Bastien,
>>
>> Bastien <bzg@gnu.org> writes:
>>
>>> Hi Diego,
>>>
>>> Diego Zamboni <diego@zzamboni.org> writes:
>>>
>>>> I'm late to the discussion so I apologize in advance, but this fix
>>>> seems counterintuitive to me. In my mind, for any shell code:
>>>>
>>>> - Return value: exit code of the last command
>>>> - Output: whatever the commands print
>>>
>>> Yes, that's what is *now* possible if you set
>>> ob-shell-return-value-is-exit-status to t.
>>>
>>> Unless I miss something, it was not possible before today.
>>>
>>> #+begin_src shell
>>> echo Hello!
>>> #+end_src
>>>
>>> would simply return "Hello!" as a return.
>>>
>>> No exit code was *never* output.
>>>
>>>> So to me, it's intuitive that =:exports value= would return the exit
>>>> code of the last command, and =:exports output= would produce the
>>>> output of the commands. I don't understand why a new option is
>>>> needed.
>>>
>>> ... because it was not the case before.  Or maybe *I* miss something.
>>>
>>> Can you show me something that was working before and that is not
>>> anymore?
>>>
>>> Thanks,
>>
>> I welcome the fix but not the option: the option situation in Org mode
>> was pretty horrible, then ca 2010, you did a survey and some options
>> were eliminated (not sure about the year or whether it *was* you who
>> did the survey, but I'm pretty sure there was one): that was the right
>> direction to go, but it wasn't enough. Org mode needs to be put into
>> an option diet, so adding another one here (and a rather gratuitous
>> one in my view) is not the way to go.
>>
>> `:results value' should *always* produce the value of the last expression,
>> which for shell programs is the exit status of the last command.
>>
>> `:results output' should *always* produce all of the output of the program.
>>
>> An argument can be made that `:results value' is the default, but it
>> is the less useful option for shell programs. So maybe for shell
>> blocks, make the default to be `:results output' instead: people get
>> what they always got before the fix without lifting a finger, the exit status
>> is now available with `:results value', and the option can go away
>> quietly and quickly, before it becomes another contributor to the Org
>> mode technical debt.


-- 
Paul Scherrer Institut
Dr. Derek Feichtinger                   Phone:   +41 56 310 47 33
Group Head HPC and Emerging Technologies  Email: derek.feichtinger@psi.ch
Building/Room No. WHGA/U126
Forschungsstrasse 111
CH-5232 Villigen PSI 

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Nick,

I didn't conduct the survey in 2013, you can find the results here:
https://orgmode.org/worg/org-configs/org-customization-survey.html

I understand why you (and Tim and Derek) think an option is too much.

But let's separate two problems: one is that Org have many options,
some perhaps unneeded (or only here because of bad design decisions we
made in the past), causing a technical debt, another one being "How to
handle shell's notion of "return value" in ob-shell.el?

The first problem is real: patches are always welcome in this area but
this is the kind of problem that requires a big picture and a strong
push, incremental fixes are difficult here.

The second problem is real too, and while being cautious about not
making the first problem worse, the solution should be independant.

That said, we have three solutions:

1. Stick to a strict reading of Org and bash manuals: the absence of a
   :results header means "return the value, i.e. the exit status".

2. Deviate from this strict reading, introduce an exception for *all*
   shell blocks: no :results header means "return output" and you need
   to use :results value to get the exit status (your proposal).

3. Deviate from this strict reading and introduce an option that says:
   "Don't deviate from the Org's and bash manuals for all src blocks".

Obviously, nobody wants the first solution.

Part of me agrees with your second solution: after all, we had to wait
until Vladimir's "echo ." trick to find out there was a problem.

Part of me think that (1) deviation from the normal behavior should be
carefully advertized and (2) by design, the normal behavior should not
require tweaking options manually for each block.  An option fulfills
both these needs: allowing to get the normal behavior by default and
advertizing the "deviation".

Right now I'm not convinced we should get rid of this option.

But I'm convinced we should take the decision we no consideration of
the first (biggest) problem of Org having *perhaps* too many options.

Case still open.

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Nick Dokos]

Hi Bastien,

I think all this is reasonable. I have some inline comments and
a suggestion at the end.

Bastien <bzg@gnu.org> writes:

> Hi Nick,
>
> I didn't conduct the survey in 2013, you can find the results here:
> https://orgmode.org/worg/org-configs/org-customization-survey.html
>
> I understand why you (and Tim and Derek) think an option is too much.
>
> But let's separate two problems: one is that Org have many options,
> some perhaps unneeded (or only here because of bad design decisions we
> made in the past), causing a technical debt, another one being "How to
> handle shell's notion of "return value" in ob-shell.el?
>
> The first problem is real: patches are always welcome in this area but
> this is the kind of problem that requires a big picture and a strong
> push, incremental fixes are difficult here.
>

But if we can avoid making the problem worse, we should do that. Messes
like this are created, not born.

> The second problem is real too, and while being cautious about not
> making the first problem worse, the solution should be independant.
>
> That said, we have three solutions:
>
> 1. Stick to a strict reading of Org and bash manuals: the absence of a
>    :results header means "return the value, i.e. the exit status".
>
> 2. Deviate from this strict reading, introduce an exception for *all*
>    shell blocks: no :results header means "return output" and you need
>    to use :results value to get the exit status (your proposal).
>
> 3. Deviate from this strict reading and introduce an option that says:
>    "Don't deviate from the Org's and bash manuals for all src blocks".
>
> Obviously, nobody wants the first solution.

I'm not convinced of that: personally, I would be happy with the first
solution and maybe others would be too. I would add a :results output
header explicitly where needed and be on my way. Of course, it would
break workflows where the output is expected and that might be a
problem for some. I agree that the option takes care of this problem.

>
> Part of me agrees with your second solution: after all, we had to wait
> until Vladimir's "echo ." trick to find out there was a problem.
>
> Part of me think that (1) deviation from the normal behavior should be
> carefully advertized and (2) by design, the normal behavior should not
> require tweaking options manually for each block.  An option fulfills
> both these needs: allowing to get the normal behavior by default and
> advertizing the "deviation".

I somewhat object to the use of the word "normal" in this paragraph: I
think you mean "current" in the first and last instances.  I disagree
with (2): see my suggestion below. And IMO, the option makes it so
that most users don't have to change anything. That is a good think in
general (it is the reason that the option exists), but it hides the
deviation rather than advertising it, because unless somebody follows
this discussion, they don't know that there *is* a deviation.

>
> Right now I'm not convinced we should get rid of this option.
>
> But I'm convinced we should take the decision we no consideration of
> the first (biggest) problem of Org having *perhaps* too many options.
>
> Case still open.

My longer term suggestion (which, I realize, does not help to close
*this* case any time soon):

I think that the global default setting `:results value' is wrong: it
works fine for "functional" code blocks (ones where a function is
defined and then called with some parameters to make sure it produces
the correct value - the function can be written in any language: I
don't mean to restrict it to functional languages); it does not work
well for the "scripting" cases (ones where the code block is a
sequence of calls, each of which might or might not produce output -
but it's the whole output that matters, not the individual calls).

In the longer run, I think the possibility of *forcing* users to choose
which one they want, should be entertained.

Thanks and regards!
-- 
Nick

"There are only two hard problems in computer science: cache
invalidation, naming things, and off-by-one errors." -Martin Fowler

Re: Bug or not a bug? dot expansion in ob-shell

[Jack Kamm]

Hi,

Nick Dokos <ndokos@gmail.com> writes:

>> That said, we have three solutions:
>>
>> 1. Stick to a strict reading of Org and bash manuals: the absence of a
>>    :results header means "return the value, i.e. the exit status".
>>
>> 2. Deviate from this strict reading, introduce an exception for *all*
>>    shell blocks: no :results header means "return output" and you need
>>    to use :results value to get the exit status (your proposal).
>>
>> 3. Deviate from this strict reading and introduce an option that says:
>>    "Don't deviate from the Org's and bash manuals for all src blocks".
>>
>> Obviously, nobody wants the first solution.
>
> I'm not convinced of that: personally, I would be happy with the first
> solution and maybe others would be too.
> 
> My longer term suggestion (which, I realize, does not help to close
> *this* case any time soon):
> 
> I think that the global default setting `:results value' is wrong

Strongly agree with Nick on both points here. I prefer option 1 over
options 2 and 3, I think it is the more consistent and simple behavior.

I also think ":results value" is a problematic default.

Anecdotally, I use Org-Babel as a computational notebook similar to
Jupyter or Rmarkdown, and set ":session :results output" for nearly all
my org-babel blocks.

Of course, there are many ways in which org-babel can be used, and for
some uses, ":results value" is the better default. But for other use
cases, including shell blocks, ":results output" is the more intuitive
default.

And I think this is really the problem here. The result of a shell block
is surprising to the new user, because the user probably wants ":results
output", but ":results value" is the default. I'm not sure of the
solution to this problem, but I don't think it's to change the meaning
of ":results value".

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Nick and Jack,

thanks for your thoughtful inputs.  I've now removed the option.

I agree we should have a discussion on whether :results value is a
good default.  But I think this needs previous work on documenting
what is the actual behavior of the main Babel libraries wrt "value"
vs "output".

I will start a file on worg with a table describing the current
behavior of various libraries.  E.g. ob-clojure.el the output by
default, not the "value".

Once we have a cleear picture on how various libraries distant
themselves from what could be expected by reading Org's manual,
then we have more background and could start the discussion on
changing the defaults - 9.5 would be a good time.

Thanks again,

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Stefan Nobis]

Bastien <bzg@gnu.org> writes:

> I agree we should have a discussion on whether :results value is a
> good default.

What about a third collection option 'none' and make this the default?

This would emphasize that there is no sensible default for all babel
languages, users and use cases. Users would be forced to explicitly
select either the 'value' or 'output' option, which also helps
reproducibility a little bit (explicit is better than implicit).

The downside would be that many existing Org documents needs to be
fixed (but maybe it would be not too hard to automate the adjustments)
and some source blocks may become a little more verbose.

Just my 2¢.

-- 
Until the next mail...,
Stefan.

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Stefan,

Stefan Nobis <stefan-ml@snobis.de> writes:

> The downside would be that many existing Org documents needs to be
> fixed (but maybe it would be not too hard to automate the adjustments)
> and some source blocks may become a little more verbose.

Yes, backward-compatibilities issues, verbosity, and a non-functional
default are problematic.

I think we will have a better view on what is a sensible default after
we try to document the current behavior for the main languages.

Also, such a documentation page will be useful even after we change
the defaults, if we do this.

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Fraga, Eric]

On Friday, 21 Feb 2020 at 09:04, Bastien wrote:

[...]

> That said, we have three solutions:
>
> 1. Stick to a strict reading of Org and bash manuals: the absence of a
>    :results header means "return the value, i.e. the exit status".

[...]

> Obviously, nobody wants the first solution.

Respectfully, I disagree!  I think the first solution is the cleanest
and most correct one.  Yes, this means a change in the behaviour but the
current behaviour, in my opinion, is wrong given org and bash
documentation.

I definitely do not like the third option (yet another variable).

Thank you.
-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-354-g9d5880

Re: Bug or not a bug? dot expansion in ob-shell

[Jack Kamm]

Hi Stefan,

Stefan Nobis <stefan-ml@snobis.de> writes:

> What about a third collection option 'none' and make this the default?

A variant on this idea, would be to instead have the third collection
option be 'default', which would stand for a language-specific
default. For example, ":results default" could be equivalent to
":results output" for shell blocks, but equivalent to ":results value"
for emacs-lisp blocks.

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> Stefan Nobis <stefan-ml@snobis.de> writes:
>
>> What about a third collection option 'none' and make this the default?
>
> A variant on this idea, would be to instead have the third collection
> option be 'default', which would stand for a language-specific
> default. For example, ":results default" could be equivalent to
> ":results output" for shell blocks, but equivalent to ":results value"
> for emacs-lisp blocks.

Before exploring other collection options, I think we should document
the current state of the default for the main Babel languages.  This
will give us an idea on what to use as the default.

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

Hello all,


On Wed, Feb 19, 2020 at 7:11 AM Bastien <bzg@gnu.org> wrote:

> Hi Eric,
>
> note that the previous behavior only _seemed_ right by chance: there
> is no notion of getting the exit code of the shell command in
> ob-shell.el, and returning "0" is just a hazard here, just because
> (org-babel--string-to-number ".") returns "0", while it should return
> nil.
>

Seems like I missed this long thread. After this change to
org-babel--string-to-number, now (org-babel--string-to-number "1,3-5") is
now returning 1 (instead of returning nil as before).

Related thread that I just started:
https://lists.gnu.org/r/emacs-orgmode/2020-02/msg00932.html

[-- Attachment #2: Type: text/html, Size: 1281 bytes --]

Re: Bug or not a bug? dot expansion in ob-shell

[Jack Kamm]

Hi Bastien,

Bastien <bzg@gnu.org> writes:

> thanks for your thoughtful inputs.  I've now removed the option.

I think it's good you removed the option.

However, it looks like the behavior now is to return the output, instead
of the exit code, when ":results value".

According to my reading of this thread, most of the commenters were in
agreement that we should keep the original behavior and return the exit
code, as we do in 9.3.

Re: Bug or not a bug? dot expansion in ob-shell

[Jack Kamm]

Sorry, I was confused about this:

> According to my reading of this thread, most of the commenters were in
> agreement that we should keep the original behavior and return the exit
> code, as we do in 9.3.

Actually, it looks like ":results value" does return the exit code. I
just got confused because shell blocks now return the output when no
":results" is specified.

Re: Bug or not a bug? dot expansion in ob-shell

[Tom Gillespie]

Hi all,
    Sorry to be late to this thread (and for a wall of text), but as a
heavy user of ob-shell I wanted to chime in. First a disclosure, I
would be very unhappy if option 1 were selected, it would require me
to make a whole bunch of changes and try to find an option to revert
to the current default behavior. When I run a bash block in org mode,
I want what is going to stdout, why else would I run it in org? If I
don't want to capture the output then it is either in a pipeline
inside the block, or I will silence the results. Option two is by far
the most reasonable answer in my opinion and there is a consistent
principle which can be applied in many cases so that it would not be
an exception. The principle is that block defaults for results should
default to the value that the language itself makes the most
accessible. Shell return codes are esoteric from this point of view, I
had been writing bash scripts for years before I learned about $?. By
far the most actionable and accessible thing to a user of a shell
program is the stdout -- ignore the naming for a moment, because the
meaning is not in the name, it is in how it interacts with other
programs in the larger environment. If we apply the logic that says
that option 1 should be the default, then python blocks set to results
value should return nothing unless an error is raised and then they
should return the error. This is clearly absurd, no python programmer
cares about the absence of an error and making the default behavior of
python blocks verbosely report their non-errorness by default would
likely incite a riot. What has happened with shells is that the
nomenclature has been switched with respect to how users and other
code consume and deal with 'outputs' aka return values in any other
langues, shell return values are error signals and should be treated
as such, unfortunately (or perhaps fortunately?) org-mode doesn't have
an error handling/signaling system, but if a python code block fails
emacs will yell at us, shells don't do this, it is up to the user to
detect and handle the error case themselves, but that is a language
internal matter. In summary, option 2 seems to me to be the most
consistent with how users interact with shell commands and shell
scripts, return codes are more akin to error handling in other
languages, so ignoring the naming issues, if `my-org-block` behave
like a shell function, then pipe would be the more appropriate default
behavior than `$?`. I think that the underlying principle can be
applied to other languages as well to arrive at sane defaults.
Thoughts?
Tom


On Sat, Feb 29, 2020 at 7:41 AM Jack Kamm <jackkamm@gmail.com> wrote:
>
> Sorry, I was confused about this:
>
> > According to my reading of this thread, most of the commenters were in
> > agreement that we should keep the original behavior and return the exit
> > code, as we do in 9.3.
>
> Actually, it looks like ":results value" does return the exit code. I
> just got confused because shell blocks now return the output when no
> ":results" is specified.
>

Re: Bug or not a bug? dot expansion in ob-shell

[Tim Cross]

It seems to me that two separate issues have been mixed up and causing
some confusion here. However, I think it is actually quite simple once
we consider the issues separately.

Issue 1: Defining the meaning of :result value and :result output
Issue 2: Specifying what the default :result setting would be i.e.
:result without a value/output specifier.

Issue 1 seems the most straight-forward to me
- output :: Whatever goes to stdout/stderr
- value :: whatever would be returned by the execution of the code. This
might be a return value (i.e. for some shell commands) or the value
returned by a function (including shell functions i.e. bash function) or
the last command executed etc. Essentially, anything returned (including
nil) by the block. STDOUT/STDERR is never a return value (though some
languages may send output to STDOUT/STDERR as well as returning it, in
which case it would also be in :return value).

Note that I don't agree that the only 'useful' result from shell blocks
is output. You might have a complex shell script which uses source
blocks to define shell functions
that return values which you use via oweb expansion to include in other
blocks etc.s

With this definition, essentially :result value is essentially anything
except whatever is sent to stdout/stderr.

Issue 2 is a little more difficult. It is likely that a 'standard'
default for :result that is the same for all languages is not
possible/desired. The default will likely be a combination of what seems
most natural for that language and what is most common usage for the
majority of users. It could be that for some languages, the default for
:result should be :result output and for others it should be :result
value.

Personally, I care less about issue 2 than issue 1. In the worst case, I
will need to change my header arguments for some blocks and that would
be easily automated. Far more critical is that :result value and :result
output are clear, unambiguous and consistent. Any proposal to change
these meanings because of different uses cases in languages is a bad
idea. Instead, changing the 'default' would be preferable. Having shell
source blocks return STDOUT/STDERR output for :result value is IMO a bad
idea. Having shell blocks default to :result output when only specifying
:result while having other languages, like python or clojure default to
:result value seems far more preferable (provided differences are
clearly documented of course).

The current situation seems inconsistent to me e.g.

#+begin_src shell :results value
  echo .

#+end_src

#+RESULTS:
: .


#+begin_src shell :results output
  echo .

#+end_src

#+RESULTS:
: .

You get the same results value regardless of whether :results value or
:results output. I think :results value should return 0 (return code for
echo). Whether shell blocks defaults to :results output or :results
value is a different questions (it probably should default to :result
output). e.g.


#+begin_src shell
  echo .

#+end_src

#+RESULTS:
: .


#+begin_src shell :results value
  echo .

#+end_src

#+RESULTS:
: 0


#+begin_src shell :results output
  echo .

#+end_src

#+RESULTS:
: .

Tom Gillespie <tgbugs@gmail.com> writes:

> Hi all,
>     Sorry to be late to this thread (and for a wall of text), but as a
> heavy user of ob-shell I wanted to chime in. First a disclosure, I
> would be very unhappy if option 1 were selected, it would require me
> to make a whole bunch of changes and try to find an option to revert
> to the current default behavior. When I run a bash block in org mode,
> I want what is going to stdout, why else would I run it in org? If I
> don't want to capture the output then it is either in a pipeline
> inside the block, or I will silence the results. Option two is by far
> the most reasonable answer in my opinion and there is a consistent
> principle which can be applied in many cases so that it would not be
> an exception. The principle is that block defaults for results should
> default to the value that the language itself makes the most
> accessible. Shell return codes are esoteric from this point of view, I
> had been writing bash scripts for years before I learned about $?. By
> far the most actionable and accessible thing to a user of a shell
> program is the stdout -- ignore the naming for a moment, because the
> meaning is not in the name, it is in how it interacts with other
> programs in the larger environment. If we apply the logic that says
> that option 1 should be the default, then python blocks set to results
> value should return nothing unless an error is raised and then they
> should return the error. This is clearly absurd, no python programmer
> cares about the absence of an error and making the default behavior of
> python blocks verbosely report their non-errorness by default would
> likely incite a riot. What has happened with shells is that the
> nomenclature has been switched with respect to how users and other
> code consume and deal with 'outputs' aka return values in any other
> langues, shell return values are error signals and should be treated
> as such, unfortunately (or perhaps fortunately?) org-mode doesn't have
> an error handling/signaling system, but if a python code block fails
> emacs will yell at us, shells don't do this, it is up to the user to
> detect and handle the error case themselves, but that is a language
> internal matter. In summary, option 2 seems to me to be the most
> consistent with how users interact with shell commands and shell
> scripts, return codes are more akin to error handling in other
> languages, so ignoring the naming issues, if `my-org-block` behave
> like a shell function, then pipe would be the more appropriate default
> behavior than `$?`. I think that the underlying principle can be
> applied to other languages as well to arrive at sane defaults.
> Thoughts?
> Tom
>
>
> On Sat, Feb 29, 2020 at 7:41 AM Jack Kamm <jackkamm@gmail.com> wrote:
>>
>> Sorry, I was confused about this:
>>
>> > According to my reading of this thread, most of the commenters were in
>> > agreement that we should keep the original behavior and return the exit
>> > code, as we do in 9.3.
>>
>> Actually, it looks like ":results value" does return the exit code. I
>> just got confused because shell blocks now return the output when no
>> ":results" is specified.
>>


--
Tim Cross

Re: Bug or not a bug? dot expansion in ob-shell

[Jack Kamm]

Hi Tom,

Tom Gillespie <tgbugs@gmail.com> writes:

> First a disclosure, I would be very unhappy if option 1 were selected,
> it would require me to make a whole bunch of changes and try to find
> an option to revert to the current default behavior.

Wasn't option 1 already the default behavior, until the changes made
earlier this month due to this thread? That is the behavior I get when I
check out earlier commits, or maint.

> The principle is that block defaults for results should default to the
> value that the language itself makes the most accessible.

I agree that this would be a good principle.

My understanding is that in 9.3 and earlier, the default was generally
":results value", but now due to this thread we are adding an exception
for ob-shell.

It sounds like after 9.4 is released, there will be a general survey of
how org-babel behaves across languages, after which there will be a
discussion of sensible default behavior.

Re: Bug or not a bug? dot expansion in ob-shell

[Tom Gillespie]

Hi Jack,
    As with many things in emacs, I sometimes feel like I'm loosing my
mind, or loosing track of just exactly what variables are set or not
set, but I could swear that for at least the past year when running
#+begin_src bash blocks with C-c C-c and no :results header set the
results are the stdout. This is also true for sh and shell language
options. Maybe that was a recent change, but I just tested with emacs
-q and (org-version) -> 9.1.9 and am getting stdout (as seen below). I
have included the results of org-submit-bug-report for reference.
Best,
Tom

#+begin_src bash
pip install --user pudb
#+end_src

#+RESULTS:
| Requirement | already | satisfied: | pudb          | in |
/usr/lib/python3.7/site-packages | (2018.1) |       |         |
| Requirement | already | satisfied: | urwid>=1.1.1  | in |
/usr/lib/python3.7/site-packages | (from    | pudb) | (2.1.0) |
| Requirement | already | satisfied: | pygments>=1.0 | in |
/usr/lib/python3.7/site-packages | (from    | pudb) | (2.5.2) |


Subject: Bug: test [9.1.9 (release_9.1.9-65-g5e4542 @
/usr/share/emacs/26.3/lisp/org/)]

Emacs  : GNU Emacs 26.3 (build 1, x86_64-pc-linux-gnu, X toolkit)
 of 2019-10-09
Package: Org mode version 9.1.9 (release_9.1.9-65-g5e4542 @
/usr/share/emacs/26.3/lisp/org/)

current state:
==============
(setq
 org-src-mode-hook '(org-src-babel-configure-edit-buffer
             org-src-mode-configure-edit-buffer)
 org-after-todo-state-change-hook '(org-clock-out-if-current)
 org-metadown-hook '(org-babel-pop-to-session-maybe)
 org-clock-out-hook '(org-clock-remove-empty-clock-drawer)
 org-html-format-inlinetask-function
'org-html-format-inlinetask-default-function
 org-odt-format-headline-function 'org-odt-format-headline-default-function
 org-ascii-format-inlinetask-function 'org-ascii-format-inlinetask-default
 org-mode-hook '(#[0 "\300\301\302\303\304$\207"
           [add-hook change-major-mode-hook org-show-block-all append
            local]
           5]
         #[0 "\300\301\302\303\304$\207"
           [add-hook change-major-mode-hook org-babel-show-result-all
            append local]
           5]
         org-babel-result-hide-spec org-babel-hide-all-hashes)
 org-odt-format-drawer-function #[514 "\207" [] 3 "\n\n(fn NAME CONTENTS)"]
 org-archive-hook '(org-attach-archive-delete-maybe)
 org-confirm-elisp-link-function 'yes-or-no-p
 org-agenda-before-write-hook '(org-agenda-add-entry-text)
 org-metaup-hook '(org-babel-load-in-session-maybe)
 org-bibtex-headline-format-function #[257 "\300 \236A\207" [:title] 3
"\n\n(fn ENTRY)"]
 org-latex-format-drawer-function #[514 "\207" [] 3 "\n\n(fn _ CONTENTS)"]
 org-babel-pre-tangle-hook '(save-buffer)
 org-tab-first-hook '(org-babel-hide-result-toggle-maybe
              org-babel-header-arg-expand)
 org-ascii-format-drawer-function #[771 " \207" [] 4 "\n\n(fn NAME
CONTENTS WIDTH)"]
 org-occur-hook '(org-first-headline-recenter)
 org-cycle-hook '(org-cycle-hide-archived-subtrees org-cycle-hide-drawers
          org-cycle-show-empty-lines
          org-optimize-window-after-visibility-change)
 org-speed-command-hook '(org-speed-command-activate
              org-babel-speed-command-activate)
 org-odt-format-inlinetask-function 'org-odt-format-inlinetask-default-function
 org-confirm-shell-link-function 'yes-or-no-p
 org-link-parameters '(("id" :follow org-id-open)
               ("rmail" :follow org-rmail-open :store
            org-rmail-store-link)
               ("mhe" :follow org-mhe-open :store org-mhe-store-link)
               ("irc" :follow org-irc-visit :store org-irc-store-link)
               ("info" :follow org-info-open :export org-info-export
            :store org-info-store-link)
               ("gnus" :follow org-gnus-open :store
            org-gnus-store-link)
               ("docview" :follow org-docview-open :export
            org-docview-export :store org-docview-store-link)
               ("bibtex" :follow org-bibtex-open :store
            org-bibtex-store-link)
               ("bbdb" :follow org-bbdb-open :export org-bbdb-export
            :complete org-bbdb-complete-link :store
            org-bbdb-store-link)
               ("w3m" :store org-w3m-store-link) ("file+sys")
               ("file+emacs") ("doi" :follow org--open-doi-link)
               ("elisp" :follow org--open-elisp-link)
               ("file" :complete org-file-complete-link)
               ("ftp" :follow
            (lambda (path) (browse-url (concat "ftp:" path))))
               ("help" :follow org--open-help-link)
               ("http" :follow
            (lambda (path) (browse-url (concat "http:" path))))
               ("https" :follow
            (lambda (path) (browse-url (concat "https:" path))))
               ("mailto" :follow
            (lambda (path) (browse-url (concat "mailto:" path))))
               ("news" :follow
            (lambda (path) (browse-url (concat "news:" path))))
               ("shell" :follow org--open-shell-link))
 org-latex-format-headline-function 'org-latex-format-headline-default-function
 org-latex-format-inlinetask-function
'org-latex-format-inlinetask-default-function
 org-html-format-drawer-function #[514 "\207" [] 3 "\n\n(fn NAME CONTENTS)"]
 org-html-format-headline-function 'org-html-format-headline-default-function
 )

On Sat, Feb 29, 2020 at 8:11 PM Jack Kamm <jackkamm@gmail.com> wrote:
>
> Hi Tom,
>
> Tom Gillespie <tgbugs@gmail.com> writes:
>
> > First a disclosure, I would be very unhappy if option 1 were selected,
> > it would require me to make a whole bunch of changes and try to find
> > an option to revert to the current default behavior.
>
> Wasn't option 1 already the default behavior, until the changes made
> earlier this month due to this thread? That is the behavior I get when I
> check out earlier commits, or maint.
>
> > The principle is that block defaults for results should default to the
> > value that the language itself makes the most accessible.
>
> I agree that this would be a good principle.
>
> My understanding is that in 9.3 and earlier, the default was generally
> ":results value", but now due to this thread we are adding an exception
> for ob-shell.
>
> It sounds like after 9.4 is released, there will be a general survey of
> how org-babel behaves across languages, after which there will be a
> discussion of sensible default behavior.

Re: Bug or not a bug? dot expansion in ob-shell

[Jack Kamm]

Hi Tom,

Tom Gillespie <tgbugs@gmail.com> writes:

> As with many things in emacs, I sometimes feel like I'm loosing my
> mind, or loosing track of just exactly what variables are set

You're not losing your mind. After some further testing I find that
you're right, and my understanding of the situation was incorrect.

The reason I got confused was the initial example that started this
thread:

> #+begin_src sh
>   echo .
> #+end_src
> 
> #+RESULTS:
> : 0

Which looks like it's returning the exit code. However, it turns out
that this is just a coincidence, and it's an entirely unrelated bug.

If I had read the thread more carefully, I would have seen Bastien had
already noted this here:
https://lists.gnu.org/archive/html/emacs-orgmode/2020-02/msg00716.html

I fear my confusion of the issues lead to some misinformed comments on
my part. Sorry about that. I think I wasn't the only commenter confused
about this...

Anyways, it seems the current behavior of ob-shell blocks is:

- If no ":results" specified: returns the output, transformed to a
  table. Same as the old ":results value".
- If ":results value" explicitly specified: returns the exit code. This
  is a change from the old default behavior.

I can see why you, and other regular users of ob-shell, would be annoyed
by this change of behavior.

Now that I understand what's going on, I think it would be better to
stick with the old behavior (no exit code), just fixing the original bug
that prompted this thread.

Re: Bug or not a bug? dot expansion in ob-shell

[Jack Kamm]

As someone who doesn't really use ob-shell, I've made too much noise in
this thread, and am intending this to be my last comment here...

I think it's a bad idea that ":results value", and not specifying
":results", give different behavior.

In my own Org-babel files, I usually have a header line like follows:

#+PROPERTY: header-args :results output

Which sets the default return value to ":results output". Then, I
manually specify ":results value" when I need that instead.

But if explicitly writing ":results value" is different than the
default, then it would no longer be possible to use this pattern
anymore.

I think it would be prudent to revert the change to ":results value"
before the release of 9.4. I think Tom makes a convincing case that
returning the exit code is not only wrong, but also disruptive to
existing users of ob-shell.

Re: Bug or not a bug? dot expansion in ob-shell

[Nick Dokos]

Hi Tim,

Tim Cross <theophilusx@gmail.com> writes:

> It seems to me that two separate issues have been mixed up and causing
> some confusion here. However, I think it is actually quite simple once
> we consider the issues separately.
>
> Issue 1: Defining the meaning of :result value and :result output
> Issue 2: Specifying what the default :result setting would be i.e.
> :result without a value/output specifier.
>
> Issue 1 seems the most straight-forward to me
> - output :: Whatever goes to stdout/stderr
> - value :: whatever would be returned by the execution of the code. This
> might be a return value (i.e. for some shell commands) or the value
> returned by a function (including shell functions i.e. bash function) or
> the last command executed etc. Essentially, anything returned (including
> nil) by the block. STDOUT/STDERR is never a return value (though some
> languages may send output to STDOUT/STDERR as well as returning it, in
> which case it would also be in :return value).
>
> Note that I don't agree that the only 'useful' result from shell blocks
> is output. You might have a complex shell script which uses source
> blocks to define shell functions
> that return values which you use via oweb expansion to include in other
> blocks etc.s
>
> With this definition, essentially :result value is essentially anything
> except whatever is sent to stdout/stderr.
>
> Issue 2 is a little more difficult. It is likely that a 'standard'
> default for :result that is the same for all languages is not
> possible/desired. The default will likely be a combination of what seems
> most natural for that language and what is most common usage for the
> majority of users. It could be that for some languages, the default for
> :result should be :result output and for others it should be :result
> value.
>
> Personally, I care less about issue 2 than issue 1. In the worst case, I
> will need to change my header arguments for some blocks and that would
> be easily automated. Far more critical is that :result value and :result
> output are clear, unambiguous and consistent. Any proposal to change
> these meanings because of different uses cases in languages is a bad
> idea. Instead, changing the 'default' would be preferable. Having shell
> source blocks return STDOUT/STDERR output for :result value is IMO a bad
> idea. Having shell blocks default to :result output when only specifying
> :result while having other languages, like python or clojure default to
> :result value seems far more preferable (provided differences are
> clearly documented of course).
> ...

Thanks for the (very clear) write-up. I can only nod my head
in violent agreement :-)

-- 
Nick

"There are only two hard problems in computer science: cache
invalidation, naming things, and off-by-one errors." -Martin Fowler

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Tim,

sorry for the late reply, and thanks a lot for the clear summary.

Tim Cross <theophilusx@gmail.com> writes:

> It seems to me that two separate issues have been mixed up and causing
> some confusion here. However, I think it is actually quite simple once
> we consider the issues separately.
>
> Issue 1: Defining the meaning of :result value and :result output

... which is already defined in Org's documentation.

> Issue 2: Specifying what the default :result setting would be i.e.
> :result without a value/output specifier.

After careful consideration, I suggest to follow the manual here,
which says that ":results value" is the default.

After 2f534294 in master, executing shell blocks with no :results
header will output the exit code, which is the "value".

> It could be that for some languages, the default for :result should
> be :result output and for others it should be :result value.

I decided not to go that way, even though I do sympathize with Tom's
argument in the very same thread.  I think Jack makes a compelling
argument about the need to let the default always be ":results value".

It seems less random to have some users being surprised that the value
of executing a shell source block is an exit code, than to make it
necessary for all users to check what is the default result for each
Babel library.

Best,

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> I think it's a bad idea that ":results value", and not specifying
> ":results", give different behavior.

Indeed.  

At first, I thought "If ':results value' is always the same as the
default, why have ':results value' at all?" and thought it'd be an
argument for having different defaults for different languages but
I see how I was wrong: expectations for defaults should be stable,
while the notion of "value" and "output" can vary from one language
to the other.

Note that I've fixed this in master, not in maint.  I may release
9.3.8 just for the sake of testing some fixes there, but 9.4 will
follow closely anyway.

Thanks for your patient input in this thread!

-- 
 Bastien

Re: Bug or not a bug? dot expansion in ob-shell

[Bastien]

Bastien <bzg@gnu.org> writes:

> At first, I thought "If ':results value' is always the same as the
> default, why have ':results value' at all?" and thought it'd be an
> argument for having different defaults for different languages but
> I see how I was wrong: expectations for defaults should be stable,
> while the notion of "value" and "output" can vary from one language
> to the other.

Sooooo!  I've thought about this again and here we are.

The default is to not break the previous behavior, which is to use the
output of shell scripts, not their exit codes.  This is because (1) I
don't like incompatible changes (2) the output is what most users want
anyway and (3) a default of :results value would break the default
behavior for src_sh{echo "this"} (and asking users to update to
src_sh[:results output]{echo "this"} is a bad option.)

If you want to stick to the :results value as the default, you can set
org-babel-shell-results-defaults-to-output to nil.  Yes, a new option,
but at least now we have a clear convention: the default for all Babel
libraries should be to stick to ":results value" when no :results
header is set (as the manual suggests) ; when deriving from this, a
library should add an option to do so.  

I strongly believe that having an option is better than implicitely
changing the default expectation when no :results header is set, it 
is more predictable for users.

I hope this all makes sense.

-- 
 Bastien