flameshot-org / flameshot

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

New Feature: now flameshot has ability to show screenshot history #3619

Open Michaelzhouisnotwhite opened 1 month ago

Michaelzhouisnotwhite commented 1 month ago

Usage

use ', ' and '. ' to get history of screenshots:

Details:

My contribution

add a singleton class: CaptureHistoryUtils

history screenshots that user export are saved in getCachePath() + /cap.his.<selection size> + <datetime>

ScreenShots

https://github.com/flameshot-org/flameshot/assets/52443370/453dc9c6-a1b9-4d1e-ba5a-d607a1a8ce05

mmahmoudian commented 3 weeks ago

Thanks for the PR. I truely appreciate it.

In general we would like to have a PR after the issue is already discussed in an issues or in our Matrix channel. This would help the maintainers to see if they think the feature is in-line with the perspective of the project, and also would help during the review process.

Apart from that, I'm extremely puzzled on what this feature does. I quickly checked the code and that didn't solve much of my confusion. Would you please elaborate on:

  1. What is the use-case of this feature
  2. How a user should use this

To me it seems you clearly have a need for such feature and you were kind enough to also push the feature to the main repo. I just don't understand what that need was.

Michaelzhouisnotwhite commented 3 weeks ago

Thanks for the PR. I truely appreciate it.

In general we would like to have a PR after the issue is already discussed in an issues or in our Matrix channel. This would help the maintainers to see if they think the feature is in-line with the perspective of the project, and also would help during the review process.

Apart from that, I'm extremely puzzled on what this feature does. I quickly checked the code and that didn't solve much of my confusion. Would you please elaborate on:

  1. What is the use-case of this feature
  2. How a user should use this

To me it seems you clearly have a need for such feature and you were kind enough to also push the feature to the main repo. I just don't understand what that need was.

Thanks for your time to review this pr.

1. What is the use-case of this feature

This feature is to be able to view the pictures that have been captured in the past. I call it back-tracking screenshot history.

For example, when I want to screenshot and copy a live screen, but the captured screen is not perfect as the screen is no longer exists. I can use this function to backtrack to the previous screen and capture the area again.

2. How a user should use this

a. open the flameshot b. type ',' or '. ' on the keyboard to see the history of screenshots

main function implement is here: image

mmahmoudian commented 3 weeks ago

Thanks for the explanation. I can see that it can be useful for some users. I also like the name you chose for it (back-tracking).

In the light of Microsoft Windows Recall and the similarities to this back-tracking feature, I have a few concerns:

  1. User should be able to toggle it on/off, and by default it should be off (such features with potential privacy implications should be Opt-in)
  2. User should be able to define how much cache they would like to keep, and the rest should be cleaned up (I think number of screenshots is better than a Megabyte cutoff, so the default I guess should be a relatively small number like 5)
  3. Would be super nice (end even essential) that the screenshots in the cache folder can be encrypted somehow and only decrypted when needed. This is something that I don't have a clear solution for it. For example:
    • Flameshot create a password-protected ed25519 pgp key pair for asymmetric encryption. The keys can be saved in the config folder of Flameshot
    • when screenshot is taken and this back-tracking feature is enabled, the public key is used to encrypt the screenshot and then saved to the cache folder. For this part, no input from user is needed as the public key does not need password to encrypt
    • when user wants to use the back-tracking to bring the previous screenshots from the cache, we can ask the user once for the password to decrypt the screenshots on the fly and show them
    • when user is done with back-tracking and closes the window of flameshot, then flameshot "forgets" the password

You can create a new tab in the config window (flameshot config) for back-tracking settings. There can be:

  1. a checkbox to toggle this feature
  2. a part for the user to define cache folder path
  3. a button to generate the pgp key pairs for the user (if the key already exists, the generate button should be disabled and a text should be be added to indicate the existence of a key)

About the encryption part we need to discuss this with some more folks and get their opinion on the best practices, but for now if you can add the tab in the config to address my first two concerns, that would be great.

In the meantime, we can also wait for the following folks to weigh in and express their opinions: @veracioux @panpuchkov @jack9603301 @hosiet @holazt @lupoDharkael

Michaelzhouisnotwhite commented 3 weeks ago

You can create a new tab in the config window (flameshot config) for back-tracking settings. There can be:

  1. a checkbox to toggle this feature
  2. a part for the user to define cache folder path
  3. a button to generate the pgp key pairs for the user (if the key already exists, the generate button should be disabled and a text should be be added to indicate the existence of a key)

About the encryption part we need to discuss this with some more folks and get their opinion on the best practices, but for now if you can add the tab in the config to address my first two concerns, that would be great.

Thanks for your suggestions. In the near future,I will complete the following needs:

  1. a checkbox to toggle this feature
  2. a part for the user to define cache folder path
  3. a spinbox to define a number of screenshot need to keep in cache.

footnote: this 'back-tracking' idea comes from anothor closed-source screenshot application named Snipaste. However, the Snipaste only avaliable on windows. Flameshot is a great app and opensource, so I want the flameshot has this feature as well. So this pr came up.

mmahmoudian commented 3 weeks ago

@Michaelzhouisnotwhite Thank you for your hard work. I highly appreciate it.

May I suggest going through the code and use a unified naming scheme for this feature? I put a small comment on one of your commits, but now that I went through the whole PR again, I realized that this is potentially a wider concern.

In the code we have these diverse terminologies which are more or less pointing to the same concept

I Understand that they are not all identical, but I suggest trying to unifying the terminology to make the maintenance of the project easier in the future. I believe the "backtrack" is a very fitting name and less confusing comparing to "rollback" or "history", because these pictures (as far as I understand them) are raw PNGs (no annotation, no cropping...).

Anyways, I feel this is a nice PR. Thank you again.

stuart938503 commented 1 week ago

While this PR wouldn't meet the original request in #240, it is related (IMO) and helps address the essence of the problem in that issue - editing screenshots after the capture window is closed.