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

feat: let user select the directory to save things #680

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

asd55667
Copy link

/claim #669
/closes #669

the summay of changes:

ui

  • add data directory button in recording-settings
  • update content of code-block in dev-mode-settings
  • update path in log-file-button and pipe-store (unverified)

tauri

  • update default path with tauri store in command load_pipe_config, save_pipe_config, reset_all_pipes (unverified)
  • add command to update_screenpipe_dir to update data_dir setting from tauri store to temp file

server

  • read tauri data_dir setting from temp file

I have no clue to check pip-store related changes works, the remains mentioned above all manually checked in my mac-air-m3 device, both dev and build. some missing may occur due to the lack of my knowledge.

Copy link

vercel bot commented Nov 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screenpipe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2024 7:25am

.ok_or_else(|| anyhow::anyhow!("failed to get home directory"))?
.join(".screenpipe");

// check .screenpipe_dir file whether it exists in default_path, if so, replace default_path
let screenpipe_dir_path = default_path.join(".screenpipe_dir");
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is .screenpipe_dir? don't recall ever using this dir

Screenshot 2024-11-14 at 9 26 54 AM

this is mine with default data dir

@louis030195
Copy link
Collaborator

louis030195 commented Nov 14, 2024

overall great contribution

i wonder if all still work well like including pipes which rely on the .screenpipe

https://docs.screenpi.pe/docs/plugins#available-pipes

i use the reddit one which rely on the dir, it writes logs of your day using AI and then search for relevant reddit posts based on these logs and generate answer to it

can you explain the .screenpipe_dir?

@asd55667
Copy link
Author

asd55667 commented Nov 14, 2024

I'm kind of busy in last commit time, the screenpipe-server.rs file which writes media data and db file seems like an external process with tauri, I couldn't find a better way to get the data_dir field from tauri store, so i choose a ungracefully way to add a . screenpipe_dir temp file for accessing the data_dir field, when data_dir changes, it will write . screenpipe_dir in, and the restart server will read from it

@asd55667
Copy link
Author

asd55667 commented Nov 15, 2024

There are also some problem with this pr. what I have found as follow:

  • when change the default_path(~/.screenpipe) to another, the default_path couldn't be changed back with current ui, filename startsWith . are invisible in mac/win.
  • the directory after changing screenpip has no permission to writein

I will handle these in another pr and verify pipes in next few days. the ui part may adjust as

  • single click on data_dir button in recording-settings to trrigle system dialog to select a directory.
  • double click on data_dir button in recording-settings to let user type a valid directory

@louis030195
Copy link
Collaborator

okay might be related

i will test this PR later

@louis030195
Copy link
Collaborator

still dont understand what is .screenpipe_dir

@louis030195
Copy link
Collaborator

louis030195 commented Nov 19, 2024

you can do this to display hidden files on macos finder

defaults write com.apple.finder AppleShowAllFiles TRUE
killall Finder

(PS: finder is such a trash software XD)

@asd55667
Copy link
Author

asd55667 commented Nov 20, 2024

still dont understand what is .screenpipe_dir

Sorry for my confusing word, let me explain in detail, when user change the data_dir in settings and saved(from ~/.screenpipe to ~/.screenpip1, i just serialize the string ~/.screenpip1 and save to ~/screenpipe/.screenpipe_dir,

$ cat ~/.screenpipe/.screenpipe_dir
# ~/.screenpip1

And then the recorder server start, it will load path from the ~/.screenpipe/.screenpipe_dir file, and write log in there.

the filename .screenpipe_dir was named from the parameter of the constructor of server in screenpipe-server/src/server.rs. When the recorder server starts, it should initalize the data_dir from settings which stored in tauri (in my laptop /Users/wcw/Library/Application Support/screenpipe/store.bin), I used to thought the store should be load from StoreBuilder of tauri, and use the .screenpipe_dir file to sync state for convenient, maybe you will find a better way to resolve it.

@louis030195
Copy link
Collaborator

@asd55667 did you look at

fn spawn_sidecar(app: &tauri::AppHandle) -> Result<CommandChild, String> {

if you set the data dir in the sidecar here like other things it would not require the .screenpipe_dir?

@asd55667
Copy link
Author

asd55667 commented Nov 21, 2024

@asd55667 did you look at

fn spawn_sidecar(app: &tauri::AppHandle) -> Result<CommandChild, String> {

if you set the data dir in the sidecar here like other things it would not require the .screenpipe_dir?

yes. in this solution, every operation of write the data_dir field of settings should sync the value in .screenpipe_dir. it's not that elegant. The only usage of .screenpipe_dir is for operation of read in screenpipe-server/src/bin/screenpipe-server.rs.

If I set the data dir in the sidecar here, I think I should sync the value to .screenpipe_dir, and then restart the recorder server or make sure this happens before the server start.

Once if i access /Users/wcw/Library/Application Support/screenpipe/store.bin by screenpipe-server/src/bin/screenpipe-server.rs, i have to validate the recorder server in other os(linux, windows) that i don't have now.

@louis030195
Copy link
Collaborator

hey i don't get what you are saying, just use the sidecar.rs like we do with all other settings

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.

[bounty] let user select the directory to save things
2 participants