feat: browser settings in the settings page #440
Conversation
Signed-off-by: Grant Linville <grant@acorn.io>
| function browserSettingsLocation() { | ||
| const homeDir = os.homedir(); | ||
| let configDir; | ||
| if (os.platform() === 'darwin') { | ||
| configDir = | ||
| process.env.XDG_CONFIG_HOME || | ||
| join(homeDir, 'Library', 'Application Support'); | ||
| } else if (os.platform() === 'win32') { | ||
| configDir = join(homeDir, 'AppData', 'Local'); | ||
| } else if (os.platform() === 'linux') { | ||
| configDir = process.env.XDG_CONFIG_HOME || join(homeDir, '.config'); | ||
| } else { | ||
| throw new Error('Unsupported platform'); | ||
| } | ||
|
|
||
| return join(configDir, 'acorn', 'browsersettings.json'); | ||
| } |
There was a problem hiding this comment.
I hate how this function is duplicated from actions/browser.tsx. But I'm not sure if there is a way for me to define it in one place and have it be callable from both places. Please let me know if there is a way to do that.
There was a problem hiding this comment.
That I personally know of, no. I've had a lot of similar headaches. Ideally we'd make a global lib package that both the Next app and electron could import. To do that right though I think we'd need the socket server and config code to be in typescript so it isn't super simple in just this PR.
| @@ -31,6 +31,7 @@ async function startServer() { | |||
| process.env.WORKSPACE_DIR = config.workspaceDir; | |||
| process.env.GPTSCRIPT_GATEWAY_URL = config.gatewayUrl; | |||
| process.env.GPTSCRIPT_OPENAPI_REVAMP = 'true'; | |||
| process.env.GPTSCRIPT_BROWSER_SETTINGS_FILE = config.browserSettingsFile; | |||
There was a problem hiding this comment.
This environment variable is read by the browser tool itself so that it knows where to look for its settings.
There was a problem hiding this comment.
Does this mean that for various tools that have settings that they will all have their own settings files?
There was a problem hiding this comment.
I suppose so, but right now the browser tool is the only tool that reads any kind of settings file for itself.
There was a problem hiding this comment.
Makes sense. No need to prematurely optimize yet, just wanted to understand for the future.
Signed-off-by: Grant Linville <grant@acorn.io>
g-linville
requested review from
thedadams,
njhale,
StrongMonkey,
iwilltry42,
ryanhopperlowe and
tylerslaton
| @@ -31,6 +31,7 @@ async function startServer() { | |||
| process.env.WORKSPACE_DIR = config.workspaceDir; | |||
| process.env.GPTSCRIPT_GATEWAY_URL = config.gatewayUrl; | |||
| process.env.GPTSCRIPT_OPENAPI_REVAMP = 'true'; | |||
| process.env.GPTSCRIPT_BROWSER_SETTINGS_FILE = config.browserSettingsFile; | |||
There was a problem hiding this comment.
Does this mean that for various tools that have settings that they will all have their own settings files?
app/settings/page.tsx
Outdated
| <Tooltip | ||
| content="Use Default Session will use your existing default Google Chrome session. When this setting is enabled, Chrome must be closed before running the browser tool." |
There was a problem hiding this comment.
nit:
Right now this goes all the way to the edge of my app and its a little hard to read.
| <Tooltip | |
| content="Use Default Session will use your existing default Google Chrome session. When this setting is enabled, Chrome must be closed before running the browser tool." | |
| <Tooltip | |
| classNames={{base: "w-1/2"}} | |
| content="Use Default Session will use your existing default Google Chrome session. When this setting is enabled, Chrome must be closed before running the browser tool." |
Could we add that max width so it looks more like this?
| function browserSettingsLocation() { | ||
| const homeDir = os.homedir(); | ||
| let configDir; | ||
| if (os.platform() === 'darwin') { | ||
| configDir = | ||
| process.env.XDG_CONFIG_HOME || | ||
| join(homeDir, 'Library', 'Application Support'); | ||
| } else if (os.platform() === 'win32') { | ||
| configDir = join(homeDir, 'AppData', 'Local'); | ||
| } else if (os.platform() === 'linux') { | ||
| configDir = process.env.XDG_CONFIG_HOME || join(homeDir, '.config'); | ||
| } else { | ||
| throw new Error('Unsupported platform'); | ||
| } | ||
|
|
||
| return join(configDir, 'acorn', 'browsersettings.json'); | ||
| } |
There was a problem hiding this comment.
That I personally know of, no. I've had a lot of similar headaches. Ideally we'd make a global lib package that both the Next app and electron could import. To do that right though I think we'd need the socket server and config code to be in typescript so it isn't super simple in just this PR.
Signed-off-by: Grant Linville <grant@acorn.io>
g-linville
dismissed stale reviews from thedadams, tylerslaton, and ryanhopperlowe
via
8292ece
|
Pushing a change dismissed the previous approvals for some reason, so reviews re-requested. |
|
Wow that was fast lol |
for #321