Combine `apt`, `pacman` & `dnf` block to `packages` block

[IshanGrover2004]

Worked on idea proposed in Issue #1964

Before

Individual blocks for checking updates

[[block]]
block = "apt"
interval = 1800
format = "$icon $count updates available"
format_singular = " $icon One update available "
format_up_to_date = " $icon system up to date "

After

Unified apt, pacman, aur, dnf blocks in packages block and deprecates the old block

[[block]]
block = "packages"
package_manager = ["apt", "pacman", "aur","dnf"]
interval = 1800
format = "$icon $apt + $pacman + $aur + $dnf = $total updates available"
format_singular = " $icon One update available "
format_up_to_date = " $icon system up to date "

I have changed the structure of the code by adding a Backend trait which we can implement in all struct for every package manager. I did try to keep the code very generic which made easy to add any package manager in future

closes #1958

[MaxVerevkin]

Let's not delete the old blocks for now, just deprecate.

[IshanGrover2004]

Let's not delete the old blocks for now, just deprecate.

Done @MaxVerevkin

Any other thing to do?

[bim9262]

I'd suggest the following changes to note the deprecated blocks (but maybe @MaxVerevkin has some other recommendations. I can open this in another PR if you want this to rebase on the change)

diff --git a/src/blocks.rs b/src/blocks.rs
index 836e65f7..4af7e6a5 100644
--- a/src/blocks.rs
+++ b/src/blocks.rs
@@ -45,11 +45,16 @@ use crate::{BoxedFuture, Request, RequestCmd};

 macro_rules! define_blocks {
     {
-        $( $(#[cfg(feature = $feat: literal)])? $block: ident $(,)? )*
+        $(
+            $(#[cfg(feature = $feat: literal)])?
+            $(#[deprecated($($dep_k: ident = $dep_v: literal),+)])?
+            $block: ident $(,)?
+        )*
     } => {
         $(
             $(#[cfg(feature = $feat)])?
             $(#[cfg_attr(docsrs, doc(cfg(feature = $feat)))])?
+            $(#[deprecated($($dep_k = $dep_v),+)])?
             pub mod $block;
         )*

@@ -58,6 +63,7 @@ macro_rules! define_blocks {
             $(
                 $(#[cfg(feature = $feat)])?
                 #[allow(non_camel_case_types)]
+                #[allow(deprecated)]
                 $block($block::Config),
             )*
             Err(&'static str, Error),
@@ -68,6 +74,7 @@ macro_rules! define_blocks {
                 match self {
                     $(
                         $(#[cfg(feature = $feat)])?
+                        #[allow(deprecated)]
                         Self::$block { .. } => stringify!($block),
                     )*
                     Self::Err(name, _err) => name,
@@ -78,6 +85,7 @@ macro_rules! define_blocks {
                 match self {
                     $(
                         $(#[cfg(feature = $feat)])?
+                        #[allow(deprecated)]
                         Self::$block(config) => futures.push(async move {
                             while let Err(err) = $block::run(&config, &api).await {
                                 if api.set_error(err).is_err() {
@@ -114,6 +122,7 @@ macro_rules! define_blocks {
                 match block_name {
                     $(
                         $(#[cfg(feature = $feat)])?
+                        #[allow(deprecated)]
                         stringify!($block) => match $block::Config::deserialize(table) {
                             Ok(config) => Ok(BlockConfig::$block(config)),
                             Err(err) => Ok(BlockConfig::Err(stringify!($block), crate::errors::Error::new(err.to_string()))),
@@ -136,6 +145,10 @@ macro_rules! define_blocks {

 define_blocks!(
     amd_gpu,
+    #[deprecated(
+        since = "0.32.4",
+        note = "The block has been deprecated in favor of the the packages block"
+    )]
     apt,
     backlight,
     battery,
@@ -163,6 +176,10 @@ define_blocks!(
     notmuch,
     nvidia_gpu,
     packages,
+    #[deprecated(
+        since = "0.32.4",
+        note = "The block has been deprecated in favor of the the packages block"
+    )]
     pacman,
     pomodoro,
     rofication,

EDIT: format fix

[IshanGrover2004]

diff --git a/src/blocks.rs b/src/blocks.rs

---

• $( $(#[cfg(feature = $feat: literal)])? $block: ident $(,)? )*
• $( $(#[cfg(feature = $feat: literal)])?
• $(#[deprecated($($dep_k: ident = $dep_v: literal),+)])?
• $block: ident $(,)? )*

  ---

Do I need to do these changes and send commit? or maybe wait for @MaxVerevkin 's reply

[bim9262]

I'd wait to see if @MaxVerevkin has any feedback/suggestions first

[MaxVerevkin]

Deprecation patch looks good to me. Just note that the next version is 0.33. And hopefully in the future we will not need to annotate everything with #[allow(deprecated)] as apparently warnings inside the defining crate are a bug - https://github.com/rust-lang/rust/issues/47219. By the way, allow(deprecated) in BlockConfig::name() is not necessary.

[IshanGrover2004]

I did the changes proposed Anyother work todo here?

[MaxVerevkin]

Anyother work todo here?

Update pacman.rs and apt.rs to reuse the same code.

[bim9262]

We should probably include dnf in this as well as that block already exists, whereas the other examples in #1964 don't exist as their own bocks yet anyway (brew and snap)

[IshanGrover2004]

Update pacman.rs and apt.rs to reuse the same code.

Do you mean to use code of blocks/packages/pacman.rs in blocks/pacman.rs and same for apt

And if it is, then do I have to create Backend trait in apt.rs and pacman.rs separately or Just have to use one in the packages.rs

[MaxVerevkin]

Do you mean to use code of blocks/packages/pacman.rs in blocks/pacman.rs and same for apt

Yes.

or Just have to use one in the packages.rs

Yes.

[IshanGrover2004]

Everything done you said @MaxVerevkin @bim9262

[MaxVerevkin]

As mentioned by @bim9262, it's better to convert dnf too in the same PR.

[IshanGrover2004]

Any other thing left to change @MaxVerevkin

[IshanGrover2004]

This PR is ready to merge i guess since no more review left @MaxVerevkin

[c3Ls1US]

Hey, great work for this feature and I agree with unifying the package manager blocks.

Though, have you already explored using libalpm for pacman's database functionality? I believe there are Rust bindings for it and am just curious why you decided not to go this way.

[MaxVerevkin]

Please add a deprecation warning for dnf too.

[IshanGrover2004]

Please add a deprecation warning for dnf too.

Done :heavy_check_mark:

[IshanGrover2004]

hey @MaxVerevkin , Do the $both works here if $both value is not set in code in block/packages

format = " $icon $pacman + $aur = $both updates available "

If not work, should i remove $both from here

[MaxVerevkin]

hey @MaxVerevkin , Do the $both works here if $both value is not set in code in block/packages

format = " $icon $pacman + $aur = $both updates available "

If not work, should i remove $both from here

Oh, yes, missed this one. This should be $total instead of $both.

[IshanGrover2004]

Oh, yes, missed this one. This should be $total instead of $both.

Done :heavy_check_mark:

[MaxVerevkin]

Thanks!