Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add notification reading support #125

Merged
merged 34 commits into from
Mar 2, 2024
Merged

add notification reading support #125

merged 34 commits into from
Mar 2, 2024

Commits on Jan 2, 2024

  1. initial commit

    albertotirla committed Jan 2, 2024
    Configuration menu
    Copy the full SHA
    694b0ac View commit details
    Browse the repository at this point in the history
  2. add initial implementation of the notification listener library and a…

    …n integration test to make sure it works
    albertotirla committed Jan 2, 2024
    Configuration menu
    Copy the full SHA
    3c2b3cf View commit details
    Browse the repository at this point in the history

Commits on Jan 3, 2024

  1. Configuration menu
    Copy the full SHA
    ed0f15d View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    344695b View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    23e5536 View commit details
    Browse the repository at this point in the history

Commits on Jan 7, 2024

  1. remove macro for a nonexistant signal on the freedesktop notification…

    …s interface, which I thought was there because I was misreading the spec
    albertotirla committed Jan 7, 2024
    Configuration menu
    Copy the full SHA
    68efe57 View commit details
    Browse the repository at this point in the history

Commits on Jan 12, 2024

  1. Configuration menu
    Copy the full SHA
    cffad8e View commit details
    Browse the repository at this point in the history
  2. tidy up code, refactor test to uncover the real problem for not getti…

    …ng notifications, which is again a signatures mismatch
    albertotirla committed Jan 12, 2024
    Configuration menu
    Copy the full SHA
    d01cb1c View commit details
    Browse the repository at this point in the history

Commits on Jan 13, 2024

  1. remove the call to fuse on the message stream, because the zbus Messa…

    …geStream type already implements FusedStream
    
    because the library shows that it maintains the required invariants itself by implementing that trait on the type exposed through its public API, we can safely assume that we don't need to call that method our selves, in fact we could even be introducing overhead of our own by doing that, so both the call and the comment was removed.
    albertotirla committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    1ff22c9 View commit details
    Browse the repository at this point in the history
  2. add an implementation of TryFrom for Notification, so that the mappin…

    …g code is easier to write and understand
    
    this made the code in the latter half of the function more readable, however now there is a problem with esthetics,
    because the code to obtain the message from the Arc<Message> involves a function
    from the standard library, and furthermore it can fail, but not with the error type I expect,
    so I had to unwrap it for the moment, which is not good because I use the ? operator everywhere else
    the code contains a comment about this, as well as a reminder to fix it later, if I have the ability or an idea to do so
    albertotirla committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    f57e530 View commit details
    Browse the repository at this point in the history
  3. refactor library so that the Notification and Action objects live in …

    …different files
    
    this makes it easy to implement future refactorings, because the code is properly scoped
    albertotirla committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    51e33f7 View commit details
    Browse the repository at this point in the history
  4. refactor test to be easier to read and make it not panic for the wron…

    …g reason
    
    the test was hanging because a while loop was used to fetch all the notifications indefinitely, even if the assertions were successful the test would still not terminate
    furthermore, the call to the freedesktop notifications proxy is commented out, because it made the test fail due to incorrect signatures, even though the notification was shown by my desktop environment's notification daemon.
    this is pending on taking a dev dependency for a crate which sends notifications correctly
    albertotirla committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    0ca4d07 View commit details
    Browse the repository at this point in the history
  5. remove the skip(1) call from the method chain of the stream

    we apparently don't get the name lost signal in the stream, perhaps because we register the rule early enough, I'm not sure. If anyone uses a different dbus implementation and the test failes for you, do notify me of that, as in that case it's an implementation difference and more investigation would be required
    As such, skipping one *valid* notification signal from the stream, thinking it's not valid, does nothing more than cause the tests to hang because two notifications are required for the stream to return the first item, so that call is no longer there
    albertotirla committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    908bfb3 View commit details
    Browse the repository at this point in the history
  6. add dev dependency to be able to send notifications correctly and mod…

    …ify the test accordingly
    
    the notify-rust crate is now a transitive dependency of odilia-notify, used for the integration test
    furthermore, the test now correctly sends a notification and should recieve it properly
    albertotirla committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    1be1da9 View commit details
    Browse the repository at this point in the history
  7. make notification be shown asyncronously

    before, the test panicked because the show method tryed to show a notification by starting an async executor, at least on linux and bsd, where this is relevant.
    now, show_async is being called and awaited, ensuring that the same executor the test is started in is being used for the notifications as well
    albertotirla committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    901064c View commit details
    Browse the repository at this point in the history

Commits on Jan 15, 2024

  1. Deserialize Notify arguments

    - Adjusts `Action` fields to correspond with what I understand specs say about
    `Notify` method call arguments.
    
    	See: [Notification specifications]():
    	Actions are sent over as a list of pairs. [...] the identifier for the
    	action and he localized string that will be displayed to the user.
    
    	Previous `Actions` fields (name, method) are meant for the invocation
    	of the `Action` (?).
    
    - Remove `RawNotifySignature<'a>`
    - Introduce `NotifyArgs` struct that holds and names the arguments to
    the `Notify` method call we are monitoring for.
    - Make sure to derive `Type` on `NotifyArgs`.
     - Adjust `TryFrom<Message> for Notification` to
     simplify deserialize and deconstruct the `Message::body`.
    
     Since we have the `NotifyArgs` struct and `NotifyArgs` has the
     `Type + Deserialize` traits, we can now simply
    
     ```Rust
     let notify_args = msg.body::<NotifyArgs>body()?;
     ```
    
     and since we are interested in just a few fields, we can selectively
     decunstruct in one go:
    
     ```Rust
     let NotifyArgs { app_name, summary, body, actions, .. } = msg.body()?;
    ```
    
    (- Format lib.rs)
    
    Test passes. Cherry-pick whatever you like.
    
    Seems to work.
    luukvanderduim committed Jan 15, 2024
    Configuration menu
    Copy the full SHA
    9b0fda3 View commit details
    Browse the repository at this point in the history

Commits on Jan 16, 2024

  1. Addresses suggestions:

    Make things lean.
    
    - Removes `Action`s altogether.
    - Remove Notify,
    - Simplifies `Notification`
    - Simplifies `TryFrom<Merssage> for Notification`
    - Adjust test, since the actions field is no longer there.
    
    The types we do not need are references in our signature in the hope that,
    should Serde deserialize these into a temporary, these are zero-copy, borrowed
    types before these are dropped.
    luukvanderduim committed Jan 16, 2024
    Configuration menu
    Copy the full SHA
    2acb237 View commit details
    Browse the repository at this point in the history

Commits on Jan 17, 2024

  1. Merge pull request #1 from luukvanderduim/type-deserialization

    the following things were done with this merge since it was opened, which deviated at my request from the original purpose
    
    * simplify the whole API
    * remove cruft from the features I thought were needed at the beginning of making this crate
    albertotirla authored Jan 17, 2024
    Configuration menu
    Copy the full SHA
    ac3c5bb View commit details
    Browse the repository at this point in the history

Commits on Jan 20, 2024

  1. add custom error type and make the stream be wrapped in Result, not t…

    …he other way around
    
    there is now a new type, called NotifyError, which will unite all the errors encountered in the operation of the function. For now, it only has variants for dbus specific errors, in this case zbus::Error and zbus::fdo::Error
    the signature of the function inside the library has been changed, from `impl Stream<Item=Result<Notification...>>`, to `Result<impl Stream<Item=Notification>, NotifyError>
    some code comments are around the return value of the function though, because there is something strange going on with pinning currently, and tests would error out, so basically the API, as given, would be unusable without that. This is only a temporary solution, hopefully. Even if not, the cost of pinning that stream in memory is relatively small all things considered
    albertotirla committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    1666245 View commit details
    Browse the repository at this point in the history
  2. Add 'odilia-notify/' from commit '1666245bb25419785c0516823dae688f570…

    …01769'
    
    git-subtree-dir: odilia-notify
    git-subtree-mainline: cf57b7e
    git-subtree-split: 1666245
    albertotirla committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    0dee1a5 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    8ebe478 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    51e0212 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    b0037c8 View commit details
    Browse the repository at this point in the history
  6. fix formatting

    albertotirla committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    d36f4da View commit details
    Browse the repository at this point in the history
  7. Avoid extraction by TryFrom<Messages> into TryFrom<Arc<Message>>,…

    … fix spelling and make code more idioma
    
    * Remove need for `Deref` by using `.as_str()` because it's clearer to the reader, and more rust idiomatic
    * Avoid extraction from `Arc` by `TryFrom<Messages>` into `TryFrom<Arc<Message>>`. As a bonus, this will return a notification even if the strong count inside the `Arc` is greather than one, mitigating the possibility that we might lose notifications if the bus is under heavy contention
    * 'Recieve' is spelt 'receive': fix comment and rename test file.
    luukvanderduim authored Jan 20, 2024
    Configuration menu
    Copy the full SHA
    7cbc88d View commit details
    Browse the repository at this point in the history
  8. Merge commit '7cbc88d1655f1668f431662dc334e262e93f4853' into notify

    merge a pull request for odilia-notify and reflect it here as well
    albertotirla committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    295761b View commit details
    Browse the repository at this point in the history

Commits on Feb 24, 2024

  1. Merge upstream in current branch

    this allows the clippy CI check to pass, and is therefore required.
    albertotirla committed Feb 24, 2024
    Configuration menu
    Copy the full SHA
    baa3fbe View commit details
    Browse the repository at this point in the history
  2. clippy: fix another error

    main.rs imported std::string::String, even though that type was subsequently not used.
    Therefore, I removed that import, in order to get clippy to not error
    albertotirla committed Feb 24, 2024
    Configuration menu
    Copy the full SHA
    0b7ed9b View commit details
    Browse the repository at this point in the history
  3. fix: odilia no longer announces notification sender

    most applications don't provide anything in the app_name field, so it is preferable that odilia doesn't try to make something up, which is mostly the empty string in my experience
    the long-term goals are to provide such information, but for now that's not possible
    of interest is the org.gtk.notifications interface, which provides an application id, but that's internal to gnome for one, and for another, I think most of the notifications recieved that way are not ment for public consumption, and are ment more like a publisher/subscriber system for gtk apps to talk to one another. Research needed in this area
    albertotirla committed Feb 24, 2024
    Configuration menu
    Copy the full SHA
    34e1328 View commit details
    Browse the repository at this point in the history
  4. Merge branch 'main' into notify

    this makes this pr mergeable into main again, and furthermore adds cli options because of the work already merged on main
    albertotirla committed Feb 24, 2024
    Configuration menu
    Copy the full SHA
    65e288a View commit details
    Browse the repository at this point in the history

Commits on Feb 25, 2024

  1. Configuration menu
    Copy the full SHA
    53e2170 View commit details
    Browse the repository at this point in the history

Commits on Mar 1, 2024

  1. add another test, fix not running tests in workspace root and tweek w…

    …orkspace configuration
    
    * a new unit test was added inside the notification module of odilia-notify. This checks that for a given manually built message, the expected answers are returned
    * codecov was complaining before because the tests inside odilia-notify were never ran inside CI, which would cause it to conclude that I introduced coverage wholes. Even though I added another test, this wouldn't have been fixed anyway
    * therefore, I added odilia-notify to the default-members list of the root workspace file, insuring that tests run for this one as well
    * I also changed the resolver of the workspace to the newer one, because cargo was warning me every time, so that's fixed as well
    * II added afew common dependencies to the workspace proper, replacing them from odilia-notify. Hopefully, now build times should be significantly smaller
    albertotirla committed Mar 1, 2024
    Configuration menu
    Copy the full SHA
    b171129 View commit details
    Browse the repository at this point in the history

Commits on Mar 2, 2024

  1. Merge branch 'main' into notify

    pulls in the relevant CI, readme and contributing changes from upstream
    albertotirla committed Mar 2, 2024
    Configuration menu
    Copy the full SHA
    c5b5ebf View commit details
    Browse the repository at this point in the history
  2. add the proper dependencies and configuration for codecov to complete…

    … all tests successfully as well
    albertotirla committed Mar 2, 2024
    Configuration menu
    Copy the full SHA
    39a6e57 View commit details
    Browse the repository at this point in the history