bash_kernel icon indicating copy to clipboard operation
bash_kernel copied to clipboard

exit code retrieval: more robust logic

Open mmesiti opened this issue 5 months ago • 4 comments

fixes #151

mmesiti avatar Aug 26 '25 14:08 mmesiti

(Note: out of date, keeping for reference)

Relevant commits that have touched this line before:

$ git log -p -G "exitcode = .*"  | grep '^commit \|^Author:\|^Date:\|exitcode ='
commit d4722361093f0c87e79ccec3d20b5b782e555c40
Author: Michele Mesiti <[email protected]>
Date:   Tue Aug 26 16:30:47 2025 +0200
-            exitcode = int(self.bashwrapper.run_command('{ echo $?; } 2>/dev/null').rstrip().split("\r\n")[0])
+            exitcode = int(self.bashwrapper.run_command('{ echo $?; } 2>/dev/null').rstrip().split("\r\n")[-1])
             exitcode = 1
commit 55b6597ef6982a2a7894649b16a92ac56f921a8a
Author: Alexandre Bezroutchko <[email protected]>
Date:   Fri Nov 24 21:09:18 2023 +0100
-            exitcode = int(self.bashwrapper.run_command('echo $?').rstrip().split("\r\n")[0])
+            exitcode = int(self.bashwrapper.run_command('{ echo $?; } 2>/dev/null').rstrip().split("\r\n")[0])
             exitcode = 1
commit 9049a84bbe53f3482c84ab7ff8b063b812ac1a48
Author: Dr. K. D. Murray <[email protected]>
Date:   Tue Apr 11 14:08:51 2023 +0200
-            exitcode = int(self.bashwrapper.run_command('echo $?').rstrip())
+            exitcode = int(self.bashwrapper.run_command('echo $?').rstrip().split("\r\n")[0])
             exitcode = 1
commit fe6de313e7dd30fce55a63921efd1caa99d104d4
Author: Steven Silvester <[email protected]>
Date:   Thu Jul 31 06:11:17 2014 -0500
-            exitcode = int(self.run_command('echo $?').rstrip())
+            exitcode = int(self.bashwrapper.run_command('echo $?').rstrip())
             exitcode = 1
commit cb0df82ba81a8deb5743a885527542e5498a0490
Author: Thomas Kluyver <[email protected]>
Date:   Thu Jun 26 17:12:02 2014 -0700
+            exitcode = int(self.run_command('echo $?').rstrip())
+            exitcode = 1

mmesiti avatar Aug 26 '25 15:08 mmesiti

I wonder if it's better to do { echo BASHKERNEL_RETCODE=$?; } 2>/dev/null and then specifically search for the line starting with BASHKERNEL_RETCODE? The [0] is because that some shell configurations will print some guff on a line before the prompt, and so will appear after the exit code (e.g. below), so I think just taking the last isn't a complete solution.

for example, conda will do:

(condaenv)
$ command blah blah
(condaenv)
$ echo $?
0
(condaenv)
$

and then we have 0 and (condaenv) as the two lines to process.

kdm9 avatar Aug 28 '25 01:08 kdm9

I see, thank you for pointing his out. I will amend this PR

mmesiti avatar Sep 01 '25 13:09 mmesiti

With this new information, I have changed the logic.

  • Finding the exit code is now made in a separate function inspired by @kdm9 's suggestions, that can be tested separately
  • conda case should be covered

I have also added a couple of test cases to test the function I have created. I am generally in favour of automated tests, but since in this particular case I am not sure how useful they can be and how much clutter they add, I put them in a separate commit so that they can be removed easily if the maintainers decide so.

mmesiti avatar Sep 03 '25 09:09 mmesiti