notify-rs / notify

🔭 Cross-platform filesystem notification library for Rust.
https://docs.rs/notify
2.77k stars 222 forks source link

Proposal: Change the serialization format of events #553

Closed dfaust closed 10 months ago

dfaust commented 10 months ago

This relates to the notify integration into Tauri. With Tauri, developers can interact with notify using JavaScript. This means that they interface with events the way they are serialized using serde.

Currently a serialized event looks like this:

{"type":{"access":{"kind":"close","mode":"write"}},"paths":[],"attrs":{}}

This makes working with events in JavaScript a bit awkward. You have to test for event kinds individually and take into account that it may be a string or object:

let kind;
let mode;
// The two checks for `any` and `other` must be done first,
// otherwise TypeScript throws an error
if (event.type === "any") {
} else if (event.type === "other") {
} else if ("access" in event.type) {
    kind = event.type.access.kind;
    switch (event.type.access.kind) {
        case "any": {
            break;
        }
        case "close": {
            mode = event.type.access.mode; // "any" | "other" | "execute" | "read" | "write"
            break;
        }
        case "open": {
            mode = event.type.access.mode; // "any" | "other" | "execute" | "read" | "write"
            break;
        }
        case "other": {
            break;
        }
    }
} else if ("create" in event.type) {
    kind = event.type.create.kind;
} else if ("modify" in event.type) {
    kind = event.type.modify.kind;
    switch (event.type.modify.kind) {
        case "any": {
            break;
        }
        case "data": {
            mode = event.type.modify.mode; // "any" | "other" | "size" | "content"
            break;
        }
        case "metadata": {
            mode = event.type.modify.mode; // "any" | "other" | "access-time" | "write-time" | "permissions" | "ownership" | "extended"
            break;
        }
        case "name": {
            mode = event.type.modify.mode; // "any" | "other" | "to" | "from" | "both"
            break;
        }
        case "other": {
            break;
        }
    }
} else if ("remove" in event.type) {
    kind = event.type.remove.kind;
}

It would be more intuitive if we would flatten the event kind:

{"type":"access","kind":"close","mode":"write","paths":[],"attrs":{}}

Using it in JavaScript would look like:

let kind;
let mode;
switch (event.type) {
    case "any": {
        break;
    }
    case "access": {
        kind = event.kind;
        switch (event.kind) {
            case "any": {
                break;
            }
            case "close": {
                mode = event.mode; // "any" | "other" | "execute" | "read" | "write"
                break;
            }
            case "open": {
                mode = event.mode; // "any" | "other" | "execute" | "read" | "write"
                break;
            }
            case "other": {
                break;
            }
        }
        break;
    }
    case "create": {
        kind = event.kind;
        break;
    }
    case "modify": {
        kind = event.kind;
        switch (event.kind) {
            case "any": {
                break;
            }
            case "data": {
                mode = event.mode; // "any" | "other" | "size" | "content"
                break;
            }
            case "metadata": {
                mode = event.mode; // "any" | "other" | "access-time" | "write-time" | "permissions" | "ownership" | "extended"
                break;
            }
            case "name": {
                mode = event.mode; // "any" | "other" | "to" | "from" | "both"
                break;
            }
            case "other": {
                break;
            }
        }
        break;
    }
    case "remove": {
        kind = event.kind;
        break;
    }
    case "other": {
        break;
    }
}

Keeping it as it is would be fine, too. But I thought that very few people would be using the serialization feature anyway. What do you think? Am I overthinking this?

@passcod I saw your comment in #487. Would such a change be a problem for you?

0xpr03 commented 10 months ago

I don't have any real stakes in this. Flattening looks fine, but I don't want to be the only voice on this.

passcod commented 10 months ago

The format is part of watchexec's public API. So long as no information is lost such that I can reconstruct it to maintain compat, I'm fine with it.