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 option to save page content with note #123

Merged
merged 4 commits into from
Feb 7, 2025

Conversation

pavka14
Copy link
Contributor

@pavka14 pavka14 commented Jan 10, 2025

  • add option to also save the page content when adding a note
  • additional option to search in the saved content and not only in notes

@Sav22999
Copy link
Owner

@pavka14 What is the use of "content" ? Why should be saved in the notes ?

@pavka14 pavka14 force-pushed the 0001_add_page_text branch from 45a4f8c to d118ebf Compare January 13, 2025 08:08
@pavka14
Copy link
Contributor Author

pavka14 commented Jan 13, 2025

@pavka14 What is the use of "content" ? Why should be saved in the notes ?

Hi, I am still working on this. It is not finished. I thought I was pushing against my fork and was only going to submit to you when done, sorry about this.

@Sav22999
Copy link
Owner

@pavka14 Ok, no problem 😄

Thanks for contributing to this project

@pavka14
Copy link
Contributor Author

pavka14 commented Jan 13, 2025

Basically, the idea is to be able to also extract the whole page text so you can search in that too, like if you later wonder "where did I read that text last year" - you will have it locally saved.

I want to expand this a lot further and to integrate it with something else I am working on. I sent you a connect request on LinkedIn so we can chat about it there.

 - add option to also save the page content when adding a note
 - additional option to search in the saved content and not only in notes
@pavka14 pavka14 force-pushed the 0001_add_page_text branch from d118ebf to 8569330 Compare January 13, 2025 09:47
@pavka14
Copy link
Contributor Author

pavka14 commented Jan 13, 2025

@Sav22999 I think it's ready for testing and review now.

Please keep in mind that I don't know any javascript, have never done anything in javascript, and all this was done using Copilot. So, if you see something stupid, probably it is stupid and not some clever hack.

In short, the purpose of the change:

  • add two options (both disabled by default): to save the visible text of a page, and to search in the saved texts
  • in future, I want to connect this to a python-based tool I am doing separately which has detailed content extraction and what not, but this is quite far off at this point.

@@ -72,6 +72,7 @@
"permissions": [
"storage",
"tabs",
"<all_urls>",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if the thing will work without this?

@Sav22999
Copy link
Owner

@pavka14 Thank you for your contribution. Because I am a bit busy in this period (til the end of the month, at least) I'll review the whole code in February... sorry for the delay

@Sav22999
Copy link
Owner

Sav22999 commented Feb 6, 2025

@pavka14 Here I am!
Tonight I'll analyse the code, finally!
Sorry for the delay 😄

Sav22999 and others added 3 commits February 7, 2025 04:46
- Restyled in All notes
- Moved from "General" to "Advanced"

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@Sav22999
Copy link
Owner

Sav22999 commented Feb 7, 2025

@pavka14 Thank you for your contribution! I'm approving this PR, after I've improved the UI and checked the code 😄

Great work!

@Sav22999 Sav22999 merged commit 4a198fc into Sav22999:main Feb 7, 2025
@Sav22999 Sav22999 mentioned this pull request Feb 7, 2025
Merged
5 tasks
@pavka14
Copy link
Contributor Author

pavka14 commented Feb 9, 2025

Thanks. I'll message you on LinkedIn about my general idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants