[RFC] Getting exit status from job with job-control #1844
Conversation
benekastah
changed the title
|
Travis failed; looks like some tests will need to be adapted as well. |
|
Well, looks like a straightforward change to me, so I think it should be okay... just a few nitpicks:
|
|
@benekastah This will definitely be useful. Can you add some tests that test non-zero exit statuses? And maybe other edge cases, if you can think of any. |
benekastah
changed the title
|
All right, I added the test for non-zero job statuses. I saw that cat was being used in that file, so I used cat. I also used bash in an effort to get a program to return an error code that wasn't just I'm not sure how to squash my commits. After pulling upstream master into my branch, my original commits don't show up when I do |
| eq({'notification', 'j', {{jobid, 'exit', 1}}}, next_message()) | ||
| nvim('command', "let j = jobstart('xxx', 'bash', ['-c', 'exit 100'])") | ||
| local jobid = nvim('eval', 'j') | ||
| eq({'notification', 'j', {{jobid, 'exit', 100}}}, next_message()) |
There was a problem hiding this comment.
How about a negative number too, and a really large number
There was a problem hiding this comment.
According to wikipedia a lot of systems (including POSIX, bash) limit exit status codes to between 0 and 255. Trying values outside of this range just seems to cause integer overflow in bash. I could use 255 instead of 100 here so that both extremes are tested.
There was a problem hiding this comment.
good point. It could be tested with unit tests that call the C functions directly, but seems like the related functions are static, so won't be easy.
|
@benekastah have you looked here? Also, are you running |
benekastah
force-pushed
the
exit-status-1795
branch
from
cbf5ba5 to
372f3c4
Compare
|
What I did was Edit: As Contributing.md suggested, I had to do a Edit 2: I was able to fix it by creating a new local branch that mirrors neovim master, cherry-picking my commit onto it, and force pushing that instead. It looks fine now, but there were tons of commits that I didn't do on here for a bit. If anyone knows why that happens, I'd love to learn. |
benekastah
force-pushed
the
exit-status-1795
branch
from
68d1712 to
4ef2fb2
Compare
|
When you pulled, did you do something like Edit: I see you've fixed it, great. |
|
Ah, nope. I just did |
|
From what I understand, I've addressed all the comments that have come up. Let me know if there's anything I missed before it can be merged. Thanks! |
|
LGTM! 👍 |
- regression by 04c0658
Update runtime files and translations. https://code.google.com/p/vim/source/detail?r=31f7581068a9c3119e3bd2cd74160eb8282c3c6e
Problem: Memory access error. (Dominique Pelle) Solution: Update tpos. (Christian Brabandt) https://code.google.com/p/vim/source/detail?r=v7-4-514
Problem: With a wrapping line the cursor may not end up in the right place.
(Nazri Ramliy)
Solution: Adjust n_extra for a Tab that wraps. (Christian Brabandt)
https://code.google.com/p/vim/source/detail?r=v7-4-517
Configuration for clang-format and related utilities.
Examples:
clang-format -style=file <file>
git clang-format -style=file <commit>
git diff -U0 HEAD^ | clang-format-diff.py -i -p1 -style=file
- @justinmk: Clarify comments - Match verbiage used in other api/*.c files - Fix a few typos/missing words
It appears that commit 393c1c59a27591d705648919b2d7fb921cba37bc (unix:
set non-block mode in uv_{pipe,tcp,udp}_open) has broken Neovim's
drawing under OSX. Let's revert to 1.2.0 until we can figure out what
is happening and get it fixed.
This makes it possible to highlight the lines starting with ~ at the end of buffers and other elements highlighted using NonText. As proposed by mhinz at https://groups.google.com/forum/#!topic/vim_dev/p3de1iU1GXI/discussion
Problem : Negative array index read @ 909.
Diagnostic : False positive.
Rationale : Suggested error path assigns a negative value to idx at
line 836 (`idx = find_command(ca.cmdchar);`). That's
impossible, as `ca.cmdchar` is set to Ctrl_BSL just two
lines above, so we know that value will be in the table.
Resolution : Assert idx >= 0.
Diagnostic : False positive.
Rationale : Coverity thinks we are forgetting to add more char to hold
NULL, but it's not taking into account that two chars from
cntxformat will no be present in the result. In fact, we
can even allocate one byte less than currently done.
Resolution : Add explanatory comment and allocate one less byte.
Marked as "Intentional" at coverity's database.
Problem : String not null terminated @ 1165.
Diagnostic : False positive.
Rationale : Code below terminates string (with NUL or '\n').
Resolution : Add explanatory comment, and assert termination.
Mark as Intentional at coverity's database.
Problem : Argument cannot be negative @ 1165.
Diagnostic : Real issue.
Rationale : len can be assigned a negative value @ 1162;
len is passed as an unsigned argument @ 1165.
Resolution : Refactor variable's types:
- Use ftello instead of ftell to avoid using long.
- Assert ftello result is safely convertible to size_t.
- Introduce variable read_size to avoid using i (int).
Problem : Unitialized scalar variable @ 3239.
Diagnostic : Harmless issue.
Rationale : It's true pos.coladd is not initialized when calling
searchit(). But that's no problem, as coladd is only set in
that function.
Resolution : Initialize variable to 0.
Disable JIT to find cause for random `PANIC: unprotected error in call to Lua API` on Travis (OS X).
- Factor out main_msg() in favor of mch_msg() and manual indentation, as to
provide a much closer representation to the actual output of '--help'.
'gcc -E' reveals that main_msg() only consists of 3 printf calls
anyways.
- Factor out for loop used for printing top part of usage text; just
print the text normally.
usage() text:
- Don't print the version; that's what '--version' is for.
- Be consistent about nomenclature, e.g. '<arg>' denotes required
argument, '-h | --help' denotes '-h' and '--help' are equivalent, etc.
- Change some instances of vim{,rc,info} to nvim
While we're here: - Remove references to the '+diff' feature, which has since been made non-optional. - Update a few Vim instances with Nvim.
The systemlist test currently calls the `echo` command which can potentially complete before being interrupted, causing random test failures. Use `yes | xargs` instead. A `yes` invocation that is not piped through `xargs` can produce a huge amount of lines in a very short time, leading memory starvation when the result is being converted into a list. `xargs` ensures only one line of output will be produced while allowing interrupt to be tested.
benekastah
force-pushed
the
exit-status-1795
branch
2 times, most recently
from
beab6d3 to
9a0101c
Compare
|
#1954 is this PR with a clean git history. Sorry for the confusion, but I couldn't fix the tons of extra commits this time around. |
Fixes #1795 by making v:job_data[2] be the exit status when the
"activity" is "exit".