Repository: alacritty/alacritty
File: alacritty/src/config/monitor.rs (Rust)

Code before:
use std::path::PathBuf;
use std::sync::mpsc;
use std::time::Duration;

use glutin::event_loop::EventLoopProxy;
use log::{debug, error};
use notify::{watcher, DebouncedEvent, RecursiveMode, Watcher};

use alacritty_terminal::thread;

use crate::event::{Event, EventType};

#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]
const DEBOUNCE_DELAY: Duration = Duration::from_millis(10);
#[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))]
const DEBOUNCE_DELAY: Duration = Duration::from_millis(1000);

pub fn watch(mut paths: Vec<PathBuf>, event_proxy: EventLoopProxy<Event>) {
    // Don't monitor config if there is no path to watch.
    if paths.is_empty() {
        return;
    }

    // Exclude char devices like `/dev/null`, sockets, and so on, by checking that file type is a
    // regular file.
    paths.retain(|path| path.metadata().map_or(false, |metadata| metadata.file_type().is_file()));

    // Canonicalize paths, keeping the base paths for symlinks.
    for i in 0..paths.len() {
        if let Ok(canonical_path) = paths[i].canonicalize() {
            match paths[i].symlink_metadata() {
                Ok(metadata) if metadata.file_type().is_symlink() => paths.push(canonical_path),
                _ => paths[i] = canonical_path,
            }
        }
    }

    // The Duration argument is a debouncing period.
    let (tx, rx) = mpsc::channel();
    let mut watcher = match watcher(tx, DEBOUNCE_DELAY) {
        Ok(watcher) => watcher,
        Err(err) => {
            error!("Unable to watch config file: {}", err);
            return;
        },
    };

    thread::spawn_named("config watcher", move || {
        // Get all unique parent directories.
        let mut parents = paths
            .iter()

Review comment (bug):
> Tests whether this file type represents a regular file. The result is mutually exclusive to the results of is_dir and is_symlink; only zero or one of these tests may pass.

This is from the Rust docs on `is_file`. This implies that this code would filter symlinks, which is incorrect. So the code should probably be `is_file() || is_symlink()`?

Code after:
use std::path::PathBuf;
use std::sync::mpsc;
use std::time::Duration;

use glutin::event_loop::EventLoopProxy;
use log::{debug, error};
use notify::{watcher, DebouncedEvent, RecursiveMode, Watcher};

use alacritty_terminal::thread;

use crate::event::{Event, EventType};

#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]
const DEBOUNCE_DELAY: Duration = Duration::from_millis(10);
#[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))]
const DEBOUNCE_DELAY: Duration = Duration::from_millis(1000);

pub fn watch(mut paths: Vec<PathBuf>, event_proxy: EventLoopProxy<Event>) {
    // Don't monitor config if there is no path to watch.
    if paths.is_empty() {
        return;
    }

    // Exclude char devices like `/dev/null`, sockets, and so on, by checking that file type is a
    // regular file.
    paths.retain(|path| {
        // Call `metadata` to resolve symbolink links.
        path.metadata().map_or(false, |metadata| metadata.file_type().is_file())
    });

    // Canonicalize paths, keeping the base paths for symlinks.
    for i in 0..paths.len() {
        if let Ok(canonical_path) = paths[i].canonicalize() {
            match paths[i].symlink_metadata() {
                Ok(metadata) if metadata.file_type().is_symlink() => paths.push(canonical_path),
                _ => paths[i] = canonical_path,
            }
        }
    }

    // The Duration argument is a debouncing period.
    let (tx, rx) = mpsc::channel();
    let mut watcher = match watcher(tx, DEBOUNCE_DELAY) {
        Ok(watcher) => watcher,
        Err(err) => {
            error!("Unable to watch config file: {}", err);
            return;
        },
    };

    thread::spawn_named("config watcher", move || {