Skip to content

Conversation

@zakiali
Copy link
Collaborator

@zakiali zakiali commented Mar 11, 2025

No description provided.

Copy link
Contributor

@ahau-square ahau-square left a comment

Choose a reason for hiding this comment

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

nice, tried it out and working for me!

.ok_or_else(|| ToolError::InvalidParameters("Missing 'path' parameter".into()))?;

let path = self.resolve_path(path_str)?;
let normalized_path = self.normalize_mac_screenshot_path(&path);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we do something like

#[cfg(target_os = "macos")]

or something that only runs this logic on mac and just skips this step in linux/windows?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea, that makes sense

Comment on lines 838 to 843
if path.exists() && path != new_path {
if let Err(e) = std::fs::rename(path, &new_path) {
eprintln!("Warning: Failed to normalize Mac screenshot filename: {}", e);
return path.to_path_buf();
}
}
Copy link
Contributor

@kalvinnchau kalvinnchau Mar 11, 2025

Choose a reason for hiding this comment

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

don't think we need the rename anymore? or did we change our mind here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removing it

@zakiali zakiali force-pushed the zaki/fix-screenshot-spacing branch 3 times, most recently from 84c4376 to 3627975 Compare March 11, 2025 20:58
@zakiali zakiali force-pushed the zaki/fix-screenshot-spacing branch from 3627975 to c49b78a Compare March 11, 2025 20:59
@zakiali zakiali merged commit ee5ef8c into main Mar 11, 2025
6 checks passed
@zakiali zakiali deleted the zaki/fix-screenshot-spacing branch March 11, 2025 21:07
@michaelneale
Copy link
Collaborator

nice catch!

michaelneale added a commit that referenced this pull request Mar 11, 2025
* origin:
  docs: Persistent Command History (#1627)
  change to make build work on windows, macos, linux (#1618)
  chore(release): release version 1.0.13 (#1623)
  fix: handle mac screenshots with the image tool (#1622)
  feat: write eval results to eval dir (#1620)
  [fix] fix model config logging to remove api key (#1619)
  fix: ensure repeating benches return to initial run-dir (#1617)
sheagcraig added a commit to sheagcraig/goose that referenced this pull request Mar 12, 2025
* upstream/main:
  feat(google_drive): move credentials into keychain, add optional fallback (block#1603)
  feat: add session list command in cli (block#1586)
  feat: google sheets support (in google drive builtin MCP server) (block#1601)
  fix: deep link opening when window is closed (block#1633)
  docs: edits to docker guide (block#1639)
  feat: ollama tool shim (block#1448)
  feat: add write approve mode (block#1628)
  ui: auto update card upon config (block#1610)
  fix: fix tool output expansion checks (block#1634)
  fix: remove conditional that breaks output display for tool calls (block#1631)
  docs: Persistent Command History (block#1627)
  change to make build work on windows, macos, linux (block#1618)
  chore(release): release version 1.0.13 (block#1623)
  fix: handle mac screenshots with the image tool (block#1622)
  feat: write eval results to eval dir (block#1620)
  [fix] fix model config logging to remove api key (block#1619)
  fix: ensure repeating benches return to initial run-dir (block#1617)
cbruyndoncx pushed a commit to cbruyndoncx/goose that referenced this pull request Jul 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants