Fix watches on moves and removes (#3371)

Co-authored-by: Laurenz <laurmaedje@gmail.com>

crates/typst-cli/src/compile.rs

@@ -63,7 +63,8 @@ impl CompileCommand {
 
 /// Execute a compilation command.
 pub fn compile(mut timer: Timer, mut command: CompileCommand) -> StrResult<()> {
-    let mut world = SystemWorld::new(&command.common)?;
+    let mut world =
+        SystemWorld::new(&command.common).map_err(|err| eco_format!("{err}"))?;
     timer.record(&mut world, |world| compile_once(world, &mut command, false))??;
     Ok(())
 }

crates/typst-cli/src/main.rs

@@ -77,7 +77,7 @@ fn print_error(msg: &str) -> io::Result<()> {
     write!(output, "error")?;
 
     output.reset()?;
-    writeln!(output, ": {msg}.")
+    writeln!(output, ": {msg}")
 }
 
 #[cfg(not(feature = "self-update"))]

crates/typst-cli/src/query.rs

@@ -16,6 +16,7 @@ use crate::world::SystemWorld;
 /// Execute a query command.
 pub fn query(command: &QueryCommand) -> StrResult<()> {
     let mut world = SystemWorld::new(&command.common)?;
+
     // Reset everything and ensure that the main file is present.
     world.reset();
     world.source(world.main()).map_err(|err| err.to_string())?;

crates/typst-cli/src/terminal.rs

@@ -1,6 +1,5 @@
 use std::io::{self, IsTerminal, Write};
 use std::sync::atomic::{AtomicBool, Ordering};
-use std::time::Duration;
 
 use codespan_reporting::term::termcolor;
 use ecow::eco_format;
@@ -18,7 +17,6 @@ pub fn out() -> TermOut {
 
 /// The stuff that has to be shared between instances of [`TermOut`].
 struct TermOutInner {
-    active: AtomicBool,
     stream: termcolor::StandardStream,
     in_alternate_screen: AtomicBool,
 }
@@ -35,7 +33,6 @@ impl TermOutInner {
 
         let stream = termcolor::StandardStream::stderr(color_choice);
         TermOutInner {
-            active: AtomicBool::new(true),
             stream,
             in_alternate_screen: AtomicBool::new(false),
         }
@@ -54,25 +51,10 @@ impl TermOut {
     /// Initialize a handler that listens for Ctrl-C signals.
     /// This is used to exit the alternate screen that might have been opened.
     pub fn init_exit_handler(&mut self) -> StrResult<()> {
-        /// The duration the application may keep running after an exit signal was received.
-        const MAX_TIME_TO_EXIT: Duration = Duration::from_millis(750);
-
         // We can safely ignore the error as the only thing this handler would do
         // is leave an alternate screen if none was opened; not very important.
         let mut term_out = self.clone();
         ctrlc::set_handler(move || {
-            term_out.inner.active.store(false, Ordering::Release);
-
-            // Wait for some time and if the application is still running, simply exit.
-            // Not exiting immediately potentially allows destructors to run and file writes
-            // to complete.
-            std::thread::sleep(MAX_TIME_TO_EXIT);
-
-            // Leave alternate screen only after the timeout has expired.
-            // This prevents console output intended only for within the alternate screen
-            // from showing up outside it.
-            // Remember that the alternate screen is also closed if the timeout is not reached,
-            // just from a different location in code.
             let _ = term_out.leave_alternate_screen();
 
             // Exit with the exit code standard for Ctrl-C exits[^1].
@@ -84,11 +66,6 @@ impl TermOut {
         .map_err(|err| eco_format!("failed to initialize exit handler ({err})"))
     }
 
-    /// Whether this program is still active and was not stopped by the Ctrl-C handler.
-    pub fn is_active(&self) -> bool {
-        self.inner.active.load(Ordering::Acquire)
-    }
-
     /// Clears the entire screen.
     pub fn clear_screen(&mut self) -> io::Result<()> {
         // We don't want to clear anything that is not a TTY.

crates/typst-cli/src/watch.rs

@@ -1,19 +1,22 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::io::{self, Write};
-use std::path::{Path, PathBuf};
+use std::iter;
+use std::path::PathBuf;
+use std::sync::mpsc::Receiver;
+use std::time::{Duration, Instant};
 
 use codespan_reporting::term::termcolor::WriteColor;
 use codespan_reporting::term::{self, termcolor};
 use ecow::eco_format;
-use notify::{RecommendedWatcher, RecursiveMode, Watcher};
+use notify::{Event, RecommendedWatcher, RecursiveMode, Watcher as _};
 use same_file::is_same_file;
 use typst::diag::StrResult;
 
 use crate::args::{CompileCommand, Input};
 use crate::compile::compile_once;
-use crate::terminal;
 use crate::timings::Timer;
-use crate::world::SystemWorld;
+use crate::world::{SystemWorld, WorldCreationError};
+use crate::{print_error, terminal};
 
 /// Execute a watching compilation command.
 pub fn watch(mut timer: Timer, mut command: CompileCommand) -> StrResult<()> {
@@ -23,127 +26,235 @@ pub fn watch(mut timer: Timer, mut command: CompileCommand) -> StrResult<()> {
         .enter_alternate_screen()
         .map_err(|err| eco_format!("failed to enter alternate screen ({err})"))?;
 
+    // Create a file system watcher.
+    let mut watcher = Watcher::new(command.output())?;
+
     // Create the world that serves sources, files, and fonts.
-    let mut world = SystemWorld::new(&command.common)?;
+    // Additionally, if any files do not exist, wait until they do.
+    let mut world = loop {
+        match SystemWorld::new(&command.common) {
+            Ok(world) => break world,
+            Err(
+                ref err @ (WorldCreationError::InputNotFound(ref path)
+                | WorldCreationError::RootNotFound(ref path)),
+            ) => {
+                watcher.update([path.clone()])?;
+                Status::Error.print(&command).unwrap();
+                print_error(&err.to_string()).unwrap();
+                watcher.wait()?;
+            }
+            Err(err) => return Err(err.into()),
+        }
+    };
 
     // Perform initial compilation.
     timer.record(&mut world, |world| compile_once(world, &mut command, true))??;
 
-    // Setup file watching.
-    let (tx, rx) = std::sync::mpsc::channel();
-    let mut watcher = RecommendedWatcher::new(tx, notify::Config::default())
-        .map_err(|err| eco_format!("failed to setup file watching ({err})"))?;
+    // Watch all dependencies of the initial compilation.
+    watcher.update(world.dependencies())?;
 
-    // Watch all the files that are used by the input file and its dependencies.
-    let mut watched = HashMap::new();
-    watch_dependencies(&mut world, &mut watcher, &mut watched)?;
+    // Recompile whenever something relevant happens.
+    loop {
+        // Wait until anything relevant happens.
+        watcher.wait()?;
 
-    // Handle events.
-    let timeout = std::time::Duration::from_millis(100);
-    let output = command.output();
-    while terminal::out().is_active() {
-        let mut recompile = false;
-        if let Ok(event) = rx.recv_timeout(timeout) {
-            let event =
-                event.map_err(|err| eco_format!("failed to watch directory ({err})"))?;
+        // Reset all dependencies.
+        world.reset();
 
-            // Workaround for notify-rs' implicit unwatch on remove/rename
-            // (triggered by some editors when saving files) with the inotify
-            // backend. By keeping track of the potentially unwatched files, we
-            // can allow those we still depend on to be watched again later on.
-            if matches!(
-                event.kind,
-                notify::EventKind::Remove(notify::event::RemoveKind::File)
-            ) {
-                // Mark the file as unwatched and remove the watch in case it
-                // still exists.
-                let path = &event.paths[0];
-                watched.remove(path);
-                watcher.unwatch(path).ok();
+        // Recompile.
+        timer.record(&mut world, |world| compile_once(world, &mut command, true))??;
+
+        // Evict the cache.
+        comemo::evict(10);
+
+        // Adjust the file watching.
+        watcher.update(world.dependencies())?;
+    }
+}
+
+/// Watches file system activity.
+struct Watcher {
+    /// The output file. We ignore any events for it.
+    output: PathBuf,
+    /// The underlying watcher.
+    watcher: RecommendedWatcher,
+    /// Notify event receiver.
+    rx: Receiver<notify::Result<Event>>,
+    /// Keeps track of which paths are watched via `watcher`. The boolean is
+    /// used during updating for mark-and-sweep garbage collection of paths we
+    /// should unwatch.
+    watched: HashMap<PathBuf, bool>,
+    /// A set of files that should be watched, but don't exist. We manually poll
+    /// for those.
+    missing: HashSet<PathBuf>,
+}
+
+impl Watcher {
+    /// How long to wait for a shortly following file system event when
+    /// watching.
+    const BATCH_TIMEOUT: Duration = Duration::from_millis(100);
+
+    /// The maximum time we spend batching events before quitting wait().
+    const STARVE_TIMEOUT: Duration = Duration::from_millis(500);
+
+    /// The interval in which we poll when falling back to poll watching
+    /// due to missing files.
+    const POLL_INTERVAL: Duration = Duration::from_millis(300);
+
+    /// Create a new, blank watcher.
+    fn new(output: PathBuf) -> StrResult<Self> {
+        // Setup file watching.
+        let (tx, rx) = std::sync::mpsc::channel();
+
+        // Set the poll interval to something more eager than the default. That
+        // default seems a bit excessive for our purposes at around 30s.
+        // Depending on feedback, some tuning might still be in order. Note that
+        // this only affects a tiny number of systems. Most do not use the
+        // [`notify::PollWatcher`].
+        let config = notify::Config::default().with_poll_interval(Self::POLL_INTERVAL);
+        let watcher = RecommendedWatcher::new(tx, config)
+            .map_err(|err| eco_format!("failed to setup file watching ({err})"))?;
+
+        Ok(Self {
+            output,
+            rx,
+            watcher,
+            watched: HashMap::new(),
+            missing: HashSet::new(),
+        })
+    }
+
+    /// Update the watching to watch exactly the listed files.
+    ///
+    /// Files that are not yet watched will be watched. Files that are already
+    /// watched, but don't need to be watched anymore, will be unwatched.
+    fn update(&mut self, iter: impl IntoIterator<Item = PathBuf>) -> StrResult<()> {
+        // Mark all files as not "seen" so that we may unwatch them if they
+        // aren't in the dependency list.
+        for seen in self.watched.values_mut() {
+            *seen = false;
+        }
+
+        // Reset which files are missing.
+        self.missing.clear();
+
+        // Retrieve the dependencies of the last compilation and watch new paths
+        // that weren't watched yet.
+        for path in iter {
+            // We can't watch paths that don't exist with notify-rs. Instead, we
+            // add those to a `missing` set and fall back to manual poll
+            // watching.
+            if !path.exists() {
+                self.missing.insert(path);
+                continue;
             }
 
-            recompile |= is_event_relevant(&event, &output);
+            // Watch the path if it's not already watched.
+            if !self.watched.contains_key(&path) {
+                self.watcher
+                    .watch(&path, RecursiveMode::NonRecursive)
+                    .map_err(|err| eco_format!("failed to watch {path:?} ({err})"))?;
+            }
+
+            // Mark the file as "seen" so that we don't unwatch it.
+            self.watched.insert(path, true);
         }
 
-        if recompile {
-            // Reset all dependencies.
-            world.reset();
+        // Unwatch old paths that don't need to be watched anymore.
+        self.watched.retain(|path, &mut seen| {
+            if !seen {
+                self.watcher.unwatch(path).ok();
+            }
+            seen
+        });
 
-            // Recompile.
-            timer
-                .record(&mut world, |world| compile_once(world, &mut command, true))??;
+        Ok(())
+    }
 
-            comemo::evict(10);
+    /// Wait until there is a change to a watched path.
+    fn wait(&mut self) -> StrResult<()> {
+        loop {
+            // Wait for an initial event. If there are missing files, we need to
+            // poll those regularly to check whether they are created, so we
+            // wait with a smaller timeout.
+            let first = self.rx.recv_timeout(if self.missing.is_empty() {
+                Duration::MAX
+            } else {
+                Self::POLL_INTERVAL
+            });
 
-            // Adjust the file watching.
-            watch_dependencies(&mut world, &mut watcher, &mut watched)?;
+            // Watch for file system events. If multiple events happen
+            // consecutively all within a certain duration, then they are
+            // bunched up without a recompile in-between. This helps against
+            // some editors' remove & move behavior. Events are also only
+            // watched until a certain point, to hinder a barrage of events from
+            // preventing recompilations.
+            let mut relevant = false;
+            let batch_start = Instant::now();
+            for event in first
+                .into_iter()
+                .chain(iter::from_fn(|| self.rx.recv_timeout(Self::BATCH_TIMEOUT).ok()))
+                .take_while(|_| batch_start.elapsed() <= Self::STARVE_TIMEOUT)
+            {
+                let event = event
+                    .map_err(|err| eco_format!("failed to watch dependencies ({err})"))?;
+
+                // Workaround for notify-rs' implicit unwatch on remove/rename
+                // (triggered by some editors when saving files) with the
+                // inotify backend. By keeping track of the potentially
+                // unwatched files, we can allow those we still depend on to be
+                // watched again later on.
+                if matches!(
+                    event.kind,
+                    notify::EventKind::Remove(notify::event::RemoveKind::File)
+                        | notify::EventKind::Modify(notify::event::ModifyKind::Name(
+                            notify::event::RenameMode::From
+                        ))
+                ) {
+                    for path in &event.paths {
+                        // Remove affected path from the watched map to restart
+                        // watching on it later again.
+                        self.watcher.unwatch(path).ok();
+                        self.watched.remove(path);
+                    }
+                }
+
+                relevant |= self.is_event_relevant(&event);
+            }
+
+            // If we found a relevant event or if any of the missing files now
+            // exists, stop waiting.
+            if relevant || self.missing.iter().any(|path| path.exists()) {
+                return Ok(());
+            }
         }
     }
-    Ok(())
-}
 
-/// Adjust the file watching. Watches all new dependencies and unwatches
-/// all previously `watched` files that are no relevant anymore.
-fn watch_dependencies(
-    world: &mut SystemWorld,
-    watcher: &mut dyn Watcher,
-    watched: &mut HashMap<PathBuf, bool>,
-) -> StrResult<()> {
-    // Mark all files as not "seen" so that we may unwatch them if they aren't
-    // in the dependency list.
-    for seen in watched.values_mut() {
-        *seen = false;
-    }
-
-    // Retrieve the dependencies of the last compilation and watch new paths
-    // that weren't watched yet. We can't watch paths that don't exist yet
-    // unfortunately, so we filter those out.
-    for path in world.dependencies().filter(|path| path.exists()) {
-        if !watched.contains_key(&path) {
-            watcher
-                .watch(&path, RecursiveMode::NonRecursive)
-                .map_err(|err| eco_format!("failed to watch {path:?} ({err})"))?;
+    /// Whether a watch event is relevant for compilation.
+    fn is_event_relevant(&self, event: &notify::Event) -> bool {
+        // Never recompile because the output file changed.
+        if event
+            .paths
+            .iter()
+            .all(|path| is_same_file(path, &self.output).unwrap_or(false))
+        {
+            return false;
         }
 
-        // Mark the file as "seen" so that we don't unwatch it.
-        watched.insert(path, true);
-    }
-
-    // Unwatch old paths that don't need to be watched anymore.
-    watched.retain(|path, &mut seen| {
-        if !seen {
-            watcher.unwatch(path).ok();
+        match &event.kind {
+            notify::EventKind::Any => true,
+            notify::EventKind::Access(_) => false,
+            notify::EventKind::Create(_) => true,
+            notify::EventKind::Modify(kind) => match kind {
+                notify::event::ModifyKind::Any => true,
+                notify::event::ModifyKind::Data(_) => true,
+                notify::event::ModifyKind::Metadata(_) => false,
+                notify::event::ModifyKind::Name(_) => true,
+                notify::event::ModifyKind::Other => false,
+            },
+            notify::EventKind::Remove(_) => true,
+            notify::EventKind::Other => false,
         }
-        seen
-    });
-
-    Ok(())
-}
-
-/// Whether a watch event is relevant for compilation.
-fn is_event_relevant(event: &notify::Event, output: &Path) -> bool {
-    // Never recompile because the output file changed.
-    if event
-        .paths
-        .iter()
-        .all(|path| is_same_file(path, output).unwrap_or(false))
-    {
-        return false;
-    }
-
-    match &event.kind {
-        notify::EventKind::Any => true,
-        notify::EventKind::Access(_) => false,
-        notify::EventKind::Create(_) => true,
-        notify::EventKind::Modify(kind) => match kind {
-            notify::event::ModifyKind::Any => true,
-            notify::event::ModifyKind::Data(_) => true,
-            notify::event::ModifyKind::Metadata(_) => false,
-            notify::event::ModifyKind::Name(_) => true,
-            notify::event::ModifyKind::Other => false,
-        },
-        notify::EventKind::Remove(_) => true,
-        notify::EventKind::Other => false,
     }
 }
 

crates/typst-cli/src/world.rs

@@ -2,14 +2,14 @@ use std::collections::HashMap;
 use std::io::Read;
 use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
-use std::{fs, io, mem};
+use std::{fmt, fs, io, mem};
 
 use chrono::{DateTime, Datelike, Local};
 use comemo::Prehashed;
-use ecow::eco_format;
+use ecow::{eco_format, EcoString};
 use once_cell::sync::Lazy;
 use parking_lot::Mutex;
-use typst::diag::{FileError, FileResult, StrResult};
+use typst::diag::{FileError, FileResult};
 use typst::foundations::{Bytes, Datetime, Dict, IntoValue};
 use typst::syntax::{FileId, Source, VirtualPath};
 use typst::text::{Font, FontBook};
@@ -54,16 +54,18 @@ pub struct SystemWorld {
 
 impl SystemWorld {
     /// Create a new system world.
-    pub fn new(command: &SharedArgs) -> StrResult<Self> {
-        let mut searcher = FontSearcher::new();
-        searcher.search(&command.font_paths);
-
+    pub fn new(command: &SharedArgs) -> Result<Self, WorldCreationError> {
         // Resolve the system-global input path.
         let input = match &command.input {
             Input::Stdin => None,
-            Input::Path(path) => Some(path.canonicalize().map_err(|_| {
-                eco_format!("input file not found (searched at {})", path.display())
-            })?),
+            Input::Path(path) => {
+                Some(path.canonicalize().map_err(|err| match err.kind() {
+                    io::ErrorKind::NotFound => {
+                        WorldCreationError::InputNotFound(path.clone())
+                    }
+                    _ => WorldCreationError::Io(err),
+                })?)
+            }
         };
 
         // Resolve the system-global root directory.
@@ -73,15 +75,18 @@ impl SystemWorld {
                 .as_deref()
                 .or_else(|| input.as_deref().and_then(|i| i.parent()))
                 .unwrap_or(Path::new("."));
-            path.canonicalize().map_err(|_| {
-                eco_format!("root directory not found (searched at {})", path.display())
+            path.canonicalize().map_err(|err| match err.kind() {
+                io::ErrorKind::NotFound => {
+                    WorldCreationError::RootNotFound(path.to_path_buf())
+                }
+                _ => WorldCreationError::Io(err),
             })?
         };
 
         let main = if let Some(path) = &input {
             // Resolve the virtual path of the main file within the project root.
             let main_path = VirtualPath::within_root(path, &root)
-                .ok_or("source file must be contained in project root")?;
+                .ok_or(WorldCreationError::InputOutsideRoot)?;
             FileId::new(None, main_path)
         } else {
             // Return the special id of STDIN otherwise
@@ -99,6 +104,9 @@ impl SystemWorld {
             Library::builder().with_inputs(inputs).build()
         };
 
+        let mut searcher = FontSearcher::new();
+        searcher.search(&command.font_paths);
+
         Ok(Self {
             workdir: std::env::current_dir().ok(),
             input,
@@ -384,3 +392,39 @@ fn decode_utf8(buf: &[u8]) -> FileResult<&str> {
     // Remove UTF-8 BOM.
     Ok(std::str::from_utf8(buf.strip_prefix(b"\xef\xbb\xbf").unwrap_or(buf))?)
 }
+
+/// An error that occurs during world construction.
+#[derive(Debug)]
+pub enum WorldCreationError {
+    /// The input file does not appear to exist.
+    InputNotFound(PathBuf),
+    /// The input file is not contained withhin the root folder.
+    InputOutsideRoot,
+    /// The root directory does not appear to exist.
+    RootNotFound(PathBuf),
+    /// Another type of I/O error.
+    Io(io::Error),
+}
+
+impl fmt::Display for WorldCreationError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            WorldCreationError::InputNotFound(path) => {
+                write!(f, "input file not found (searched at {})", path.display())
+            }
+            WorldCreationError::InputOutsideRoot => {
+                write!(f, "source file must be contained in project root")
+            }
+            WorldCreationError::RootNotFound(path) => {
+                write!(f, "root directory not found (searched at {})", path.display())
+            }
+            WorldCreationError::Io(err) => write!(f, "{err}"),
+        }
+    }
+}
+
+impl From<WorldCreationError> for EcoString {
+    fn from(err: WorldCreationError) -> Self {
+        eco_format!("{err}")
+    }
+}