zshell-img-tools #104
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: zach/z-bar-qt#104
Reference in New Issue
Block a user
Delete Branch "zshell-img-tools"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Added new binary
zshell-img-toolsto add post-processing to screenshots taken with the shell's region picker.Settings now has a new category to configure the effects applied either using manual or auto mode where auto will pull the values set in your Hyprland config to mimic your decoration style for windows. Manual is self explanatory, you set the values for the effects which can also be configured in the settings category.
Which brings me to that Hyprland config fetching has been fixed, too.
Add a description and add me as a reviewer once it is ready for testing. I will look into it then.
I do not know rust at all, so code review is limited for rust. I know some basic cpp, but it looked fine. As far as I can read, qml changes are mostly config changes or changes to fit hyprland's changes.
Features I have tested
What I found
I do wish there was an issue linked to this PR. Makes it easier what feature was not within scope. I only found #109, which I assume is not within scope of this PR. I will mention what I found regardless of what I assume is part of the work.
Screenshot app/feature
Swappy app
Settings window
Enable effectsbool should hide the effects settings below.Effects modedoes do this, which is good.Corner radiusandEnable rounded cornersare separated and confusing. My suggested order (afterCorner radius):Enable rounded corners->Corner radius->Enable shadow->Shadow blur amount-> last two remain the same.For context, see image.
Binary
What works
Screenshot tool effects.
Battery toast #106
Hypr config fetching with game mode
@@ -1,12 +1,12 @@import Quickshell.IoJsonObject {Should the default value of screenshot effects be enabled? I can see a reason for either option.
@@ -154,6 +163,7 @@ fn rgba_image_to_pixmap(img: &RgbaImage) -> Pixmap {pixmap}// ShadowThis is a nitpick, perhaps a pet peeve of mine; Is it not obvious that
shadow_imgis a shadow above at line 94? If you need a comment for the contents ofpixmap_to_rgba_image, then do the contents not need better naming to reflect its function?@@ -137,3 +126,3 @@let image_path = image_path.context("Missing --image <path>")?;let config = config::Config::load().context("Failed to load config")?;// Check if any arguments were providedcli_args_providedsays the same as the comment.Also resolve the Rust formatting check (it shows checkmark, but in details, it did not pass).
I'll be working on options being greyed out when effects are disabled.
Swappy takes from your own config in ~/.config/swappy/config.json so anything it does or doesn’t do can be changed there.
Do you want this to be editable from the settings window or is knowing that enough?
Settings window
As Zach already said he’ll look into the out greying of the options
The names indeed still need to be changed.
These are still old as we already changed them in the backend.
Lastly agreed they should be reordered.
Binary
On standalone execution of the binary whenever it fails it should give feedback. I’d like to see whatever happens for you and try and solve it then.
Make sure you made packaged when using the /usr/bin/zshell-img-tools else run it from zshell/plugins/zshell-img-tools/target/release/zshell-img-tools.
Screenshot.qml
Personally as it is a feature of the bar I think it should be enabled by default.
Rust
Honestly when writing this I had horrible comments purely so I didn’t have to skim over it and immediately know as a mental note what it was for.
They could be made clearer or removed entirely.
I’ll go over it tomorrow.
Swappy was discussed. Not much to do with the complaints. The delay seems exclusive to my laptop. Other things are indeed config related.
Shadow limits are considered for the reasons to have guardrails against bugs (accidental huge number can be ugly).
Settingswindow will be resolved indeed.
The issue with the binary is its mystery outside the project where it is used. What does it do when ran outside of its designed environment? There is no
--helptooltip or flags. For example, runningzshell-cliwill suggest the user to check--help. Another example istlpfor (laptop) power management. Running tlp does nothing executable apart from printing information. I think this is important when it is not intended to run the command standalone.If it is enabled by default was discussed. Conclusion was that it is enabled by default on auto mode (taking what the user has in hypr config). The advantage is that if the user has no shadows and rounding, it will not include them (as if it is disabled). Solid for both ends.
It looks partially vibe coded. That is ok if so. Cleanup would be appreciated. But know, naming things is hard regardless of my comments on naming functions/variables.
The binary is something that will be worked on separately. As it is a tool for zshell but we give the user access to it outside of it as well. I'll create an issue with your comments that you made from this PR as enhancements for the tool itself.
For this PR however it is not mandatory for the standalone binary to be complete as the focus right now is mainly the bar.
Rustfmt lint & format failing
Lint format failing is because rustfmt is running on rust edition 2015 on base.
I specifically want to write it in the newest edition which is 2024.
If we were to change the lint format we'd be forcing us to use 2024 on every rust project within zshell.
Instead I created .rustfmt.toml` to tell rustfmt that specifcally zshell-img-tools is written in edition 2024.
If we want to globally force ourselves to write it this way then we should discuss that first.
Removed unnecessary comments
Found a new bug which will be added to #109
I assume you mean 2025?
Rust codebase still has comments. We can create issues resolving the issue I remarked about the binary and code cleanup. Code works fine which for now is fine.
I leave it up for @zach to merge. Settingswindow hides settings items when disabled. Only limiter to shadow to do, or make a separate issue to tackle this.