Commit graph

74 commits

Author SHA1 Message Date
Fabian Homborg
ef96a6614b Update termsize before a sigwinch handler
This could have been one iteration off, e.g.

```fish
function on-winch --on-signal winch
    echo $LINES
end
```

Resize the terminal, it'll print e.g.

24

then run `echo $LINES` interactively, it might have a different answer.

This isn't beautiful, but it works. A better solution might be to make
the termsize vars electric and just always update them on read?
2021-04-14 17:27:53 +02:00
Mahmoud Al-Qudsi
11951a245f Optimize pruning of job/proc exit handlers
Pre-emptively delete the handler while we have possession of the lock
before calling the event itself. It's crude, but it works.
2021-03-05 22:40:06 -06:00
Mahmoud Al-Qudsi
e7398c0248 Prune job exit handlers after running
While pid values may be reused, it is logical to assume that fish event
handlers coded against a particular job or process id mean just the job
that is currently referred to be any given pid/pgrp rather than in
perpetuity.

This trims the list of registered event handlers nice and early, and as
a bonus avoids the issue described in #7721.

The cleanup song-and-dance is extremely ugly due to the repeated locking
and unlocking of the event handler list.

Closes #7221.
2021-03-05 22:32:57 -06:00
Fabian Homborg
289bce2f25 Add event flog
I needed this, and it should be there.

[ci skip]
2020-10-06 17:25:45 +02:00
Johannes Altmanninger
de9874e4de Remove some useless casts
I think the warnings from -Wuseless-cast are mostly platform-specific but
I hope these are correct.
2020-09-08 22:44:03 +02:00
ridiculousfish
c1abb474c2 Remove some dead code and enable a test 2020-08-09 15:05:16 -07:00
ridiculousfish
dff4f140b0 Make the list of blocked events const
These are events that have been queued but not yet fired. There's no
reason to modify the events after creating them. Mark them as const
to ensure that doesn't happen.
2020-07-19 12:03:10 -07:00
ridiculousfish
2a4c545b21 Rework how signals trigger cancellation
When fish receives a "cancellation inducing" signal (SIGINT in particular)
it has to unwind execution - for example while loops or whatever else that
is executing. There are two ways this may come about:

1. The fish process received the signal
2. A child process received the signal

An example of the second case is:

    some_command | some_function

Here `some_command` is the tty owner and so will receive control-C, but
then fish has to cancel function execution.

Prior to this change, these were handled uniformly: both would just set a
cancellation signal inside the parser. However in the future we will have
multiple parsers and it may not be obvious which one to set the flag in.
So instead distinguish these cases: if a process receives SIGINT we mark
the signal in its job group, and if fish receives it we set a global
variable.
2020-07-12 12:16:01 -07:00
ridiculousfish
bc702ccb31 Fix interactive --on-signal INT handlers
f8ba0ac5bf introduced a bug where INT handlers would themselves be
cancelled, due to the signal. Defer processing handlers until the
parser is ready to execute more fish script.

Fixes the interactive case of #6649.
2020-03-01 13:31:59 -08:00
ridiculousfish
6bf9ae9aeb Fix up --on-job-exit caller
The `function --on-job-exit caller` feature allows a command substitution
to observe when the parent job exits. This has never worked very well - in
particular it is based on job IDs, so a function that observes this will
run multiple times. Implement it properly.

Do this by having a not-recycled "internal job id".

This is only used by psub, but ensure it works properly none-the-less.
2020-02-08 16:23:25 -08:00
ridiculousfish
93fc0d06d4 Rename event_type_t::job_exit to event_type_t::caller_exit
"job_exit" events, despite their name, can only be created via
the '--on-job-exit caller' misfeature of function. Rename it to make it
clear that this event type is specifically for caller-exit.
2020-02-08 16:08:26 -08:00
ridiculousfish
fba3c83ba5 Eliminate yet more calls to principal_parser()
In particular, remove job_t::from_job_id
2020-02-08 12:47:13 -08:00
ridiculousfish
f1f97b6476 Eliminate more calls to principal_parser()
Require a parser to get a job from its pgid.
2020-02-08 12:46:56 -08:00
Fabian Homborg
349b9e9dee Remove commented out debugs 2020-01-19 14:54:53 +01:00
Dan Zimmerman
8e17d29e04 Introduce the internal jobs for functions
This PR is aimed at improving how job ids are assigned. In particular,
previous to this commit, a job id would be consumed by functions (and
thus aliases). Since it's usual to use functions as command wrappers
this results in awkward job id assignments.

For example if the user is like me and just made the jump from vim -> neovim
then the user might create the following alias:
```
alias vim=nvim
```
Previous to this commit if the user ran `vim` after setting up this
alias, backgrounded (^Z) and ran `jobs` then the output might be:
```
Job	Group	State	Command
2	60267	stopped	nvim  $argv
```
If the user subsequently opened another vim (nvim) session, backgrounded
and ran jobs then they might see what follows:
```
Job	Group	State	Command
4	70542	stopped	nvim  $argv
2	60267	stopped	nvim  $argv
```
These job ids feel unnatural, especially when transitioning away from
e.g. bash where job ids are sequentially incremented (and aliases/functions
don't consume a job id).

See #6053 for more details.

As @ridiculousfish pointed out in
https://github.com/fish-shell/fish-shell/issues/6053#issuecomment-559899400,
we want to elide a job's job id if it corresponds to a single function in the
foreground. This translates to the following prerequisites:

- A job must correspond to a single process (i.e. the job continuation
    must be empty)
- A job must be in the foreground (i.e. `&` wasn't appended)
- The job's single process must resolve to a function invocation

If all of these conditions are true then we should mark a job as
"internal" and somehow remove it from consideration when any
infrastructure tries to interact with jobs / job ids.

I saw two paths to implement these requirements:

- At the time of job creation calculate whether or not a job is
  "internal" and use a separate list of job ids to track their ids.
  Additionally introduce a new flag denoting that a job is internal so
  that e.g. `jobs` doesn't list internal jobs
  - I started implementing this route but quickly realized I was
    computing the same information that would be computed later on (e.g.
    "is this job a single process" and "is this jobs statement a
    function"). Specifically I was computing data that populate_job_process
    would end up computing later anyway. Additionally this added some
    weird complexities to the job system (after the change there were two
    job id lists AND an additional flag that had to be taken into
    consideration)
- Once a function is about to be executed we release the current jobs
  job id if the prerequisites are satisfied (which at this point have
  been fully computed).
  - I opted for this solution since it seems cleaner. In this
  implementation "releasing a job id" is done by both calling
  `release_job_id` and by marking the internal job_id member variable to
  -1. The former operation allows subsequent child jobs to reuse that
  same job id (so e.g. the situation described in Motivation doesn't
  occur), and the latter ensures that no other job / job id
  infrastructure will interact with these jobs because valid jobs have
  positive job ids. The second operation causes job_id to become
  non-const which leads to the list of code changes outside of `exec.c`
  (i.e. a codemod from `job_t::job_id` -> `job_t::job_id()` and moving the
   old member variable to a non-const private `job_t::job_id_`)

Note: Its very possible I missed something and setting the job id to -1
will break some other infrastructure, please let me know if so!

I tried to run `make/ninja lint`, but a bunch of non-relevant issues
appeared (e.g. `fatal error: 'config.h' file not found`). I did
successfully clang-format (`git clang-format -f`) and run tests, though.
This PR closes #6053.
2019-12-31 10:08:50 -08:00
ridiculousfish
c19407ab0f Default parser_t::eval()'s block type to top
This is the parameter value at every call site except one. Just make it the
default.
2019-12-22 16:27:03 -08:00
ridiculousfish
a59f35a378 Make block_type_t an enum class 2019-12-22 15:37:14 -08:00
Rosen Penev
586ac3dfa7 [clang-tidy] Convert loops to range based
Found with modernize-loop-convert

Signed-off-by: Rosen Penev <rosenp@gmail.com>
2019-11-25 14:50:40 -08:00
Rosen Penev
0dfa7421f3 [clang-tidy] Convert C casts to C++ ones
Found with google-readability-casting

Signed-off-by: Rosen Penev <rosenp@gmail.com>
2019-11-25 14:17:49 -08:00
ridiculousfish
a7f1d2c0c7 Add support for fish_trace variable to trace execution
This adds support for `fish_trace`, a new variable intended to serve the
same purpose as `set -x` as in bash. Setting this variable to anything
non-empty causes execution to be traced. In the future we may give more
specific meaning to the value of the variable.

The user's prompt is not traced unless you run it explicitly. Events are
also not traced because it is noisy; however autoloading is.

Fixes #3427
2019-11-02 14:40:57 -07:00
ridiculousfish
82eca4bc86 Run clang-format on all files
The main change here is to reorder headers.
2019-10-13 15:50:48 -07:00
ridiculousfish
4a2c709fb1 Eliminate shell_is_interactive
We used to have a global notion of "is the shell interactive" but soon we
will want to have multiple independent execution threads, only some of
which may be interactive. Start tracking this data per-parser.
2019-06-29 11:28:26 -07:00
ridiculousfish
8254342339 Remove the useless second parameter from signal_handle
It was always set to 1.
2019-06-28 10:33:03 -07:00
ridiculousfish
bc103c2ea6 Make the list of event handlers thread safe 2019-06-03 12:33:10 -07:00
ridiculousfish
ead16958b2 Make set_signal_observed thread-safe 2019-06-03 12:32:48 -07:00
ridiculousfish
ad301ab3a0 Remove an incorrect shadowing use of principal_parser 2019-06-03 02:55:36 -07:00
ridiculousfish
ff55249447 Make events per-parser
This makes the following changes:

1. Events in background threads are executed in those threads, instead of
being silently dropped

2. Blocked events are now per-parser instead of global

3. Events are posted in builtin_set instead of within the environment stack

The last one means that we no longer support event handlers for implicit
sets like (example) argv. Instead only the `set` builtin (and also `cd`)
post variable-change events.

Events from universal variable changes are still not fully rationalized.
2019-06-03 02:48:35 -07:00
ridiculousfish
b405b979ec Eliminate the CHECK() macro
This thing was pretty useless.
2019-05-27 17:24:19 -07:00
ridiculousfish
eff4873eca Stop creating subclasses of block_t
Move all block_t creation methods to static methods, and stop creating
subclasses (all of which are now empty).
2019-05-19 14:40:35 -07:00
ridiculousfish
508c3a8005 Make is_event and other globals part of parser_t libdata 2019-05-18 19:03:45 -07:00
ridiculousfish
234c97e6d2 Remove some unused variables 2019-05-12 18:23:00 -07:00
ridiculousfish
1719d6f136 Make $status and $pipestatus per-parser
Another step towards allowing multiple parsers to execute in parallel.
2019-05-12 14:00:44 -07:00
Fabian Homborg
c2970f9618 Reformat all files
This runs build_tools/style.fish, which runs clang-format on C++, fish_indent on fish and (new) black on python.

If anything is wrong with the formatting, we should fix the tools, but automated formatting is worth it.
2019-05-05 12:09:25 +02:00
Aaron Gyes
2e4948e1f4 Fix switch nesting in handler_matches
I guess this worked, but whoops.
2019-03-12 15:27:13 -07:00
Aaron Gyes
d5ac239f68 This commit changes wchar.h includes to cwchar, and uses std::
for everything it provides.
2019-03-12 15:09:36 -07:00
Aaron Gyes
2ae6e5a585 Explicitly handle all enum values in more switch statements
This addresses a few places where -Wswitch-enum showed one or two missing
case's for enum values.

It did uncover and fix one apparent oversight:

$ function asd -p 100
   echo foo
end

$ functions --handlers-type exit
Event exit
asd

It looks like this should be showing a PID before 'asd' just like
job_exit handlers show the job id. It was falling
through to default: which just printed the function name.

$ functions --handlers-type exit
Event exit
100 asd
2019-03-11 15:02:18 -07:00
David Adam
b443808452 event.cpp: die if invalid event type passed
Fixes the build with -Wreturn-type.
2019-03-05 07:27:56 +08:00
ridiculousfish
1a4bb50cd5 Combine status and pipestatus into statuses_t
In most places where we set one, we want to set both. Make this less
error-prone by combining them into a single type statuses_t.
2019-02-26 20:07:37 -08:00
ridiculousfish
2c3214cabd Make $pipestatus thread safe and other misc cleanup 2019-02-24 23:29:33 -08:00
zabereer
2c8abdf5cb add $pipestatus support 2019-02-24 21:46:52 -08:00
ridiculousfish
9715db9434 Fix a "no return value" warning 2019-02-23 14:09:17 -08:00
ridiculousfish
130f2266d0 Remove the last of the signal blocking and checks
fish's signal handlers are now sufficiently innocuous that there should
be no reason to block signals (outside of temporarily, when creating a
thread and we need to manipulate the signal mask).
2019-02-23 14:07:35 -08:00
ridiculousfish
f1b208997c Cleanup events
Prior to this fix, an "event" was used as both a predicate on which events
to match, and also as the event itself. Re-express these concepts
distinctly: an event is something that happened, an event_handler is the
predicate and name of the function to execute.
2019-02-23 13:33:12 -08:00
ridiculousfish
780b53ba73 Convert event_type_t to an enum class 2019-02-23 13:17:28 -08:00
ridiculousfish
1b8ddacfed Reimplement signal handling event machinery
Prior to this fix, fish had a signal_list_t that accumulated signals.
Signals were added to an array of integers, with an overflow flag.
The event machinery would attempt to atomically "swap in" the other list.

After this fix, there is a single list of pending signal events, as an array
of atomic booleans. The signal handler sets the boolean corresponding to its
signal.
2019-02-23 13:03:33 -08:00
ridiculousfish
0f30f05e16 Remove EVENT_ANY_SIGNAL
This appeared to have been intended to allow functions to handle all signals,
but this has not been exposed and it doesn't seem useful.
2019-02-23 12:19:59 -08:00
Mahmoud Al-Qudsi
f9118d964e Clean up job flags, status helpers, and instance helper methods
* Convert JOB_* enums to scoped enums
* Convert standalone job_is_* functions to member functions
* Convert standalone job_{promote, signal, continue} to member functions
* Convert standolen job_get{,_from_pid} to `job_t` static functions
* Reduce usage of JOB_* enums outside of proc.cpp by using new
  `job_t::is_foo()` const helper methods instead.

This patch is only a refactor and should not change any functionality or
behavior (both observed and unobserved).
2018-10-27 18:01:38 -05:00
ridiculousfish
1473f952d4 Make contains() more general
Allow contains() to find arbitrary values in arbitrary vectors, and adopt it
in place of std::find.
2018-08-11 22:23:20 -07:00
ridiculousfish
9a5afe3913 Clean up and document functions --handlers 2018-03-10 02:27:25 -08:00
Benoit Hamon
3819437e0e add option --handlers to functions to display function hooks 2018-03-10 02:18:46 -08:00