AgregoreWeb / agregore-browser

A minimal browser for the distributed web (Desktop version)
https://www.youtube.com/watch?v=TnYKvOQB0ts&list=PL7sG5SCUNyeYx8wnfMOUpsh7rM_g0w_cu&index=14
GNU Affero General Public License v3.0
715 stars 66 forks source link

bookmarking failing when executable is an appimage #185

Closed av8ta closed 2 years ago

av8ta commented 2 years ago
Create-Desktop-Shortcuts:
LINUX filePath (with type of "Application") must exist and cannot be a folder: "/usr/bin/agregore.AppImage" undefined
_________________________
Create-Desktop-Shortcuts:
No shortcuts were created due to lack of accurate details passed in to options object { verbose: true, onlyCurrentOS: true }

So it looks like an appimage is considered a folder. I thought they were zip files? Anywhoo, the problem is here:

https://github.com/nwutils/create-desktop-shortcuts/blob/fa3fd0f99d1b728f2e265dc475d3b6087c10cfba/src/validation.js#L231

if (
      (!type || type === 'Application') &&
      (
        !options.linux.filePath ||
        typeof(options.linux.filePath) !== 'string' ||
        !fs.existsSync(options.linux.filePath) ||
        fs.lstatSync(options.linux.filePath).isDirectory()
      )
    ) {
      helpers.throwError(options, 'LINUX filePath (with type of "Application") must exist and cannot be a folder: ' + options.linux.filePath);
      delete options.linux;

With these two lines:

!fs.existsSync(options.linux.filePath) ||
fs.lstatSync(options.linux.filePath).isDirectory()

removed bookmarks are created successfully and open correctly too. I'll raise an issue in the dependency github.

av8ta commented 2 years ago

Issue raised at create-desktop-shortcuts

av8ta commented 2 years ago

In the log the filePath looked fine but in the object it's a string inside a string. Unsurprisingly, that doesn't work!

{
  linux: {
    filePath: '"/usr/bin/agregore.AppImage"',
    outputPath: '/home/av8ta/Downloads',
    name: "Mauve's Blog - Home",
    comment: 'Agregore Browser - hyper://94f0cab7f60fcc2a711df11c85db5e0594d11e8a3efd04a06f46a3c34d03c418/',
    description: 'Agregore Browser - hyper://94f0cab7f60fcc2a711df11c85db5e0594d11e8a3efd04a06f46a3c34d03c418/',
    arguments: 'hyper://94f0cab7f60fcc2a711df11c85db5e0594d11e8a3efd04a06f46a3c34d03c418/',
    type: 'Application'
  },
  verbose: true,
  onlyCurrentOS: true
}
av8ta commented 2 years ago

It's not related to this is it? https://github.com/AgregoreWeb/agregore-browser/issues/165 Being open that issue likely hasn't been implemented though.

The offending line: https://github.com/AgregoreWeb/agregore-browser/blob/550eadb038be0140fd4639f660197d63a9a3a365/app/actions.js#L197

Was this done for windows and their silly-paths-with-spaces-in-them? Safe on other platforms if I set a normal string here @RangerMauve ?

RangerMauve commented 2 years ago

Oh nice, yeah. A PR which updates the dependency once it gets built in quotes and changing that line would be great1

Ty for looking into this

av8ta commented 2 years ago

Done and pushed to master :pray:

RangerMauve commented 2 years ago

Sweet, ty for fixing this up!