-
Notifications
You must be signed in to change notification settings - Fork 4k
Update office-converter extension #19471
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
base: main
Are you sure you want to change the base?
Conversation
- .gitignore - Almost ready for publish - Initial commit
- Fix double execution with StrictMode - Fix convert failing if file exists
Congratulations on your new Raycast extension! 🚀 You can expect an initial review within five business days. Once the PR is approved and merged, the extension will be available on our Store. |
hi @pernielsentikaer! following up from your review in the previous PR: I can't reproduce the issue - the dropdown is shown if no Finder windows are open, or if no file is selected. Could you try again with the latest build? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
New Office Converter extension that enables converting office files to various formats using LibreOffice, featuring both general format conversion and PDF-specific conversion commands.
- Missing
metadata
folder with screenshots for view commands - please add as per Raycast Documentation - Error handling in
convert-file.ts
should useshowFailureToast
from@raycast/utils
instead of manual error handling launchCommand
inconvert-file.ts
andconvert-to-pdf.ts
should be wrapped in try-catch blocks- Remove
console.log
statements fromconvert-file.ts
before production - Add
subtitle
to commands inpackage.json
with the service name (Office Converter)
💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
14 file(s) reviewed, 18 comment(s)
Edit PR Review Bot Settings | Greptile
} catch (e) { | ||
console.error(e); | ||
failed++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using showFailureToast(error, { title: "Could not convert file" })
from @raycast/utils
for better error handling
} catch (e) { | |
console.error(e); | |
failed++; | |
} catch (e) { | |
showFailureToast(e, { title: "Could not convert file" }); | |
failed++; |
} catch (e) { | ||
console.error(e); | ||
failed++; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Use showFailureToast
from @raycast/utils
instead of manual error handling. See https://developers.raycast.com/utilities/functions/showfailuretoast
} catch (e) { | |
console.error(e); | |
failed++; | |
} | |
} catch (e) { | |
await showFailureToast(e, { title: "Failed to convert file" }); | |
failed++; | |
} |
if (selectedFiles && selectedFiles.length > 0 && format) { | ||
await handleConversion({ inputPaths: selectedFiles, format: format || "" }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Potential bug: format || ""
will allow empty string formats through validation
if (selectedFiles && selectedFiles.length > 0 && format) { | |
await handleConversion({ inputPaths: selectedFiles, format: format || "" }); | |
if (selectedFiles && selectedFiles.length > 0 && format) { | |
await handleConversion({ inputPaths: selectedFiles, format }); |
"commands": [ | ||
{ | ||
"name": "convert-file", | ||
"title": "Convert Files", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider adding subtitle
with the service name (Office Converter) since there are multiple commands
return; | ||
} | ||
|
||
console.log("Input files: ", files); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Remove console.log before publishing
export default function convertToPDF() { | ||
return convertFiles({ arguments: { format: "pdf" } }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: launchCommand should be wrapped in try-catch for proper error handling
export default function convertToPDF() { | |
return convertFiles({ arguments: { format: "pdf" } }); | |
export default async function convertToPDF() { | |
try { | |
return await convertFiles({ arguments: { format: "pdf" } }); | |
} catch (error) { | |
showFailureToast(error, { title: "Failed to convert to PDF" }); | |
} | |
} |
inputPaths: values.inputPaths as string[], | ||
format: values.format as string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Type assertions should be avoided. Use runtime validation to ensure values are of correct type
<Form.FilePicker | ||
id="inputPaths" | ||
title="Select Files" | ||
allowMultipleSelection | ||
defaultValue={props.arguments.inputFiles || []} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider adding allowedTypes prop to FilePicker to restrict to supported office file formats
allowMultipleSelection | ||
defaultValue={props.arguments.inputFiles || []} | ||
/> | ||
<Form.TextField id="format" title="Output Format" defaultValue={format} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Add placeholder text to help users understand expected format (e.g. 'pdf, docx, etc')
title="Convert" | ||
onSubmit={async (values) => { | ||
await props.onSubmit({ | ||
inputPaths: values.inputPaths as string[], | ||
format: values.format as string, | ||
}); | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Missing validation before submission - format field could be empty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Add office-converter extension: Convert office files to various formats using LibreOffice.
LibreOffice must be installed for the commands to function.
Screencast
no screenshots available; all commands are
no-view
Previous PR: #17573
Checklist
npm run build
and tested this distribution build in Raycastassets
folder are used by the extension itselfREADME
are placed outside of themetadata
folder