search

flameshot-org / flameshot

Powerful yet simple to use screenshot software :desktop_computer: :camera_flash:
https://flameshot.org
GNU General Public License v3.0
24.02k stars 1.53k forks source link

Implement basic functionality: detect a single-line box around mouse cursor. #3418

Closed libertyteeth closed 7 months ago

libertyteeth commented 7 months ago

Implement basic functionality: detect a single-line box around mouse cursor.

This addresses Issue #3416 (I am new at this, please guide me if I’m making mistakes).

Invoke with "flameshot gui --box", while the mouse cursor is inside a box, preferably over a background color.

I changed it to "box" instead of "post" as that's more general.

This is my first "large" contribution so please let me know if there's anything I missed? Thanks! (I had wanted to comment on the above issue, but it appears if I comment it will close it, which wasn’t what I wanted. So, making the Pull Request first, instead.)

ISSUES:

  1. Note that I have not translated any strings that I added into other languages.

  2. I have not updated documentation.

  3. I have not tested on multiple monitors.

FUTURE:

  1. Want to add the two features I mentioned in Issue #3416: allow for rounded corners by ignoring a percentage, or a certain number of pixels, of the ends of the lines being detected; and, an option to keep detecting until a border of a certain size has been found.

  2. Want to add the "background color detection" mentioned in Issue #3416: basically, count the pixels of each different color inside the box being checked, and the highest count is the background. (For efficiency: save the result and then only add the "box/lines just inside the four edges" to the calculation, similar to how the line-detection code only checks for the first and last pixels of an already-detected line as it grows in place.)

  3. Want to add another icon/button, so the user can click that, then click anywhere on the screen, and have it perform the box detection and auto-select a region. In other words, freeze the screen first, then select. One use case is: hover over a link which causes a popup to appear, and the user wants to capture that popup. Trying to move the mouse inside the box to be captured, though, moves the mouse off the link, and the popup goes away. So to capture those, will need this additional feature.

GENERAL ISSUES:

  1. I'd like to merge the strings in the code with the strings in flameshot.fish; there's duplication there, and the risk is that someone may update one, and not the other. Can they pull from a common source? (Probably I should create an "Issue" about this?)
libertyteeth commented 7 months ago

My code is occasionally causing my Linux user to log out. This is frustrating as it also exits all my applications, and my workspaces need to be set up again. I don't want others to have this same experience, so I am closing this Pull Request.

Also, some code I checked in to my repo after creating the PR, appears to be part of the diff for the PR. Am still learning; will do the work on a separate branch solely for this new feature.

Sorry about any confusion I might have caused. Appreciate any feedback you might have. Thanks!

libertyteeth commented 7 months ago

Thought it was my code causing Linux to log my user out. Turns out it's something that was added recently here, as I reverted to the stock Flameshot and just tried to take a screenshot which locked up for a second, and then logged me out. Will open an issue, good to know it's not something I had done.

mmahmoudian commented 7 months ago

@libertyteeth thanks for the PR. We are all learning everyday 🤓 thanks for being a responsible person and transparently discussing the issues you have found. When your changes are ready, you can create a PR as a draft and get some feedback and have some constructive discussion 👍👍