fix: Dragging and dropping files in to OSX dock icon

[9mm]

What kind of change does this PR introduce?

Currently when a file is dragged over the Neovide dock icon, it doesn't change appearance (indicating it doesn't accept drag and drop). Furthermore, when the file is dragged over (after allowing extensions) it still didn't accept the drag event so is intending to be fixed too.

• fixes Feature request: Suppport app argument to open files or folders when we drag drop them #696
• partially fixes Enable Drag and Drop on all platforms #330

There are some other issues that could potentially be fixed as well if someone with Windows wants to help. It's possible that drag and dropping on windows doesn't work for a similar reason?

• potentially fixes No externalfile drag possible on self compiled binary but works on released binary (windows) #1796

I actually think maybe this is related too, as thats the purpose of the massive CF type bundle is to say every type thats supported as an "Editor" role: #1682

Did this PR introduce a breaking change?

• No

[9mm]

I made this PR as a draft before getting it working, so I can save all my notes here and get feedback on best approach, ie: how to handle the cargo bundle not supporting it. Can we just fork that or something, and merge one of those PR's? or is there a better approach

[fredizzimo]

I think we are missing some information here. What does cargo bundle not support? And which PRs are you talking about?

[9mm]

Basically in order to have drag and drop support on OSX, it looks like in the Info.plist you need to have a key called CFBundleDocumentTypes.

https://stackoverflow.com/a/14527463/794481

I checked the bundled Neovide.app and it's missing that item from Info.plist

I tested this myself by adding the keys that are in MacVim and it didn't fix the issue completely, but it DID at least cause the icon to "accept files" which is 70% of the way there to fixing the problem. At that point I suspect the problem lies within the event handling part, or something else in the actual codebase. I didn't look that far yet. As of right now OSX doesn't even accept dragging a file period, the icon never changes appearance and if you let go of a file on the icon, nothing happens.

Given that this Info.plist isn't anywhere in the repo, I am assuming it's coming from here at build time:

https://github.com/neovide/neovide/blob/main/Cargo.toml#L106-L118

I am assuming cargo-bundle is using that to generate the Info.plist.

https://github.com/burtonageo/cargo-bundle?tab=readme-ov-file#general-settings

^ In their docs, they only support a small variety of keys that can be added. There's several complaints in all the Github issues about how he isn't maintaining it with new types, and theres months old open PR's where he's also not adding new types.

Heres an example of a PR adding a key: burtonageo/cargo-bundle#123

That one doesn't help us just showing that there's no actual way to just write your own keys/values which seems crazy to me.

I also could be missing something obvious here... but otherwise I don't know how we can write the proper keys to the Info.plist, as full MacOS support requires a lot of identifiers which are missing, for example all the right click "open with" context menus, setting a program as a default when opening, etc, are all controlled via that.

[9mm]

burtonageo/cargo-bundle#33

^ Other people wanting keys added, and him saying he doesn't have time to work on it anymore

[fredizzimo]

Ah, I see. I will search around if there are some alternatives to cargo bundle. The status of the projects says "very early alpha", and if the maintainer doesn't have time to work on it, then we have to look for alternatives.

[9mm]

I only did a rudimentary 5 mins of googling so this may not end up being any good, but maybe "tauri-bundler" which looks to be based on cargo-bundler

It looks like tauri is a full build system which wouldn't be desired, but maybe tauri-bundler can just be used separately

Im trying to see though if it supports arbitrary keys... still searching

[fredizzimo]

I checked Alacritty and they seem to be doing it manually with a makefile and hdiutil https://github.com/alacritty/alacritty/blob/master/Makefile

And with the icons and plist included in the repository https://github.com/alacritty/alacritty/tree/master/extra/osx/Alacritty.app/Contents

So, I guess that's one way to go. But not sure if it's the best.

I will do more searching later.

[9mm]

https://github.com/tauri-apps/tauri/blob/2032228cad0de6500616ca765af5c9ff1f231f0f/tooling/bundler/src/bundle/macos/app.rs#L223

It looks like it supports it through file_associations, even though i dont see it on their docs (yet)

However i did see this:

https://tauri.app/v1/guides/building/macos

These options generate the application bundle Info.plist file. You can extend the generated file with your own Info.plist file stored in the Tauri folder (src-tauri by default). The CLI merges both .plist files in production, and the core layer embeds it in the binary during development.

It looks like it might support other OS's as well (maybe not related, I'm not sure how linux bundles work)

https://tauri.app/v1/guides/building/linux#custom-files

[9mm]

But yes - whatever you think. This bit is outside my wheelhouse haha

[fredizzimo]

Maybe we could use the tool from MacVim https://github.com/macvim-dev/macvim/tree/master/src/MacVim/create-dmg

[9mm]

That might be a great idea, I can certainly say MacVim has dialed in the native Mac experience, that's for sure. It's almost too much how they're dialed in 🤣 For example, they're basically in every single right click menu, context menu, top-bar menu. Not that we need all that but good to know it would be supported.

[shaunsingh]

burtonageo/cargo-bundle#33

^ Other people wanting keys added, and him saying he doesn't have time to work on it anymore

I think it's in our best interest to move away from cargo-bundle for macOS at the least. Tauri builder (https://github.com/tauri-apps/tauri/tree/dev/tooling/bundler) at the least has not just support for Info.plist, but shipping a dmg instead of our current .app natively. I can't speak for other platforms, but I imagine there are additional bug fixes.

If everything looks good, I'll whip up a PR to move the build process over to tauri-builder. Hopefully that should make this PR a bit easier to implement.

[9mm]

I'm good wil tauri bundler but I'll defer to @fredizzimo - it looks like there's slight improvements on Linux + Windows as well, maybe some other issues would be fixed by that (ie, drag and drop on other OS's, etc)

[fredizzimo]

I would personally go with the MacVim's tool, since it seems more generic, while the tauri bundler seems specialized for their usecase of packaging tauri applications.

I'm also not sure if the cargo.toml support is there in the tauri bundler anymore, they mentioned that they refactored into a library, and I can only see the library documentation. So if the support is there, it seems to be undocumented. And their CLI tool only seem to be able to build tauri applications.

But if @shaunsingh, can get that to work, and you figure out how to enable the plist properies you need, then it should be fine.

But for example it looks like you have to pass the file associations in a settings struct to enable what you want https://github.com/tauri-apps/tauri/blob/6892a8cbc1df39c48c3b83f3700b873cbad086ed/tooling/bundler/src/bundle/macos/app.rs#L221

[9mm]

Ah very good points, well I'm good with Macvim's obviously as well too, it does what we need in this issue and likely others for sure

[fredizzimo]

For that one, I think you could try it yourself, you just need to build a release build, and give the right parameters the path to the executable, the path to the icons and so on.

[9mm]

Awesome, I might have some questions but I'll give it a go

[9mm]

@fredizzimo this may be a dumb question but will this be able to be automated? It says it requires Mac OSX and I thought all your build stuff happens in github

https://github.com/macvim-dev/macvim/tree/master/src/MacVim/create-dmg#requirements

[fredizzimo]

Yes the macOS CI runs on this https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md

[fredizzimo]

I also just noticed that the official repository for create-dmg is https://github.com/create-dmg/create-dmg

[9mm]

Awesome, I'm workin on it already I'll let you know what i come up with.

[fredizzimo]

Btw, if I understand it correctly,this is the source folder macvim use, but you only need a few things from there, for example the plist with our own modifications

https://github.com/macvim-dev/macvim/tree/master/src/MacVim

[fredizzimo]

Actually here's the command they use https://github.com/macvim-dev/macvim/blob/1a8aef0b01ce606f17115b3e71d4864f0640d1b2/src/Makefile#L3679-L3694

[fredizzimo]

Sorry, after studying more, it seems to be more complex than that 😢

The plist needs to be compiled into the executable https://github.com/macvim-dev/macvim/blob/1a8aef0b01ce606f17115b3e71d4864f0640d1b2/src/MacVim/MacVim_xcode8.xcodeproj/project.pbxproj#L261 And that's probably partly what those bundle tools does

[9mm]

Yes I was kind of coming to that conclusion at the same time, it also seems like it's just for dmg's after you already have the .app built

[fredizzimo]

Hm. based on the Alacritty makefile, I don't see anything special, it copies the executable to the same folder as the plist and renames it to Alacritty.app. So the Alacritty.app goes into a folder with the following structure https://github.com/alacritty/alacritty/tree/3d7d81c8482eb9465763020a397290765b70b541/extra/osx/Alacritty.app/Contents

Then it creates the dmg from that https://github.com/alacritty/alacritty/blob/3d7d81c8482eb9465763020a397290765b70b541/Makefile

[9mm]

Interesting, well I can make a rough version in a single file, and I will try to cobble it together and then someone perhaps could refine it, but ill get the core parts of it working first, if you think thats good approach

[abhillman]

Just an FYI -- this compiles, but does not deliver said functionality on my box as currently authored.

% uname -a
Darwin MBP.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:54:05 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6031 arm64

[abhillman]

Ideally, a change that would enable document handling for macOS might be upstreamed in cargo-bundle (e.g. burtonageo/cargo-bundle#174).

[9mm]

@abhillman
it doesnt offer said functionality because it's a draft. namely there is no good way using winit to access application:openFile:

I'm not an OSX developer so I dont have the skills to find a way for Neovide to hook into the separate event loop.

If you read the comments you will see theres multiple event loops that need to be handled, there were some possible ways with fruitbasket, etc, but I couldn't get them to work.

I have already done a lot of the heavy lifting which was a PRECURSOR to this which was adding an installer DMG and supporting the necessary Info.plist attributes.

But as far as hooking into that event, I'm at a loss.

Which is a bummer because I'd really love this functionality. I work with dozens of CSV's every day on my desktop and i have no easy way to open them. I have to keep macvim on there to use it for that use case... which sux

[fredizzimo]

Also,if you read the first messages, you will find the reason why we decided to move away from cargo bundle

The project status is

Very early alpha. Expect the format of the [package.metadata.bundle] section to change, and there is no guarantee of stabili

And that combined with very sporadic development and useful pull requests getting left open without any comments.

[9mm]

^ yes that too. TBH it would be really cool to ship this issue eventually as I did spend a lot of time making a nice installer etc, which separates the good OSX from the chaff

heres also what it is stuck on it seems:

rust-windowing/winit#1751

Would definitely be cool to find another way though, as who knows how far away that is

[abhillman]

Also,if you read the first messages, you will find the reason why we decided to move away from cargo bundle

The project status is

Very early alpha. Expect the format of the [package.metadata.bundle] section to change, and there is no guarantee of stabili

And that combined with very sporadic development and useful pull requests getting left open without any comments.

Although cargo-bundle is in alpha stage, it has thus-far served the needs of neovide. It would be a real gift to the rust community and rust's cross-platform story to add this functionality to cargo-bundle. They accepted a PR last week; though indeed, they are lagging in terms of releases to crates.io.

[9mm]

@abhillman i doubt cargo-bundle will work any time soon given the sheer amount of new keys required, unless they specify some way to allow arbitrary key entry. You can see there are a huge amount of new keys here that added:

https://github.com/neovide/neovide/pull/2191/files#diff-971b2ac9922c3c9ca5b99bb2125e7bfdd78cd3175e349608c4d723f7aba023d8

[abhillman]

@abhillman i doubt cargo-bundle will work any time soon given the sheer amount of new keys required, unless they specify some way to allow arbitrary key entry. You can see there are a huge amount of new keys here that added:

https://github.com/neovide/neovide/pull/2191/files#diff-971b2ac9922c3c9ca5b99bb2125e7bfdd78cd3175e349608c4d723f7aba023d8

Let's put aside the cargo-bundle thing for now! I think however you and other folks think is best is ideal. That said, I played around with this for a bit and wanted to share something. The minimal required addition to a plist file to allow for a document to be dropped on top of a app bundle is the following:

<key>CFBundleDocumentTypes</key>
<array>
  <dict>
    <key>LSItemContentTypes</key>
    <array>
      <string>public.data</string>
    </array>
  </dict>
</array>

In addition to this piece and other logistics, what remains is registering a message handler for the OS.

[abhillman]

Here's a sketch of how to bring this much of the rest of the way.

Background

• winit, the library responsible for creating a window for neovide, produces an AppDelegate

• the way that you would get this to work (i.e. dropping documents on an app bundle's icon and calling a method) using Objective-C is by adding the instance method application:openFiles: to a project's AppDelegate.m:

- (void)application:(NSApplication *)sender openFiles:(NSArray *) fileNames {
    NSLog(@"Files dragged on: %@", fileNames);
}

• Objective-C has a runtime that has advanced reflection and related runtime features (i.e. classes can be fetched by name, methods can be added, etc.)

Sketch of the solution

use std::ffi::CString;

use objc::runtime::{
    objc_getClass
};

unsafe fn add_url_handler() {
    let class_name = CString::new("WinitApplicationDelegate").expect("CString::new failed");
    let res = objc_getClass(class_name.as_ptr());

    // TODO: use objc::runtime::class_addMethod to add a `application:openFiles:` method to the application delegate via the Objective-C Runtime
    // TODO: tell neovim to open the file
}

[abhillman]

For a simple example demonstrating fetching winit's ApplicationDelegate and causes a window to be opened on macOS -- which can be used as a kind of simple harness for getting things to work:

use std::ffi::CString;
use winit::{
    event_loop::EventLoop,
    window::WindowBuilder
};

use objc::runtime::{
    objc_getClass
};
use winit::event::Event;

unsafe fn add_url_handler() {
    let class_name = CString::new("WinitApplicationDelegate").expect("CString::new failed");
    let res = objc_getClass(class_name.as_ptr());

    // Prints a memory address like 0x6000019d85a0
    println!("{:?}", res);
}

fn main() {
    let event_loop = EventLoop::new().unwrap();
    let _window = WindowBuilder::new().build(&event_loop).unwrap();

    let loop_proxy = event_loop.create_proxy();
    loop_proxy.send_event(()).unwrap();

    event_loop.run(|event, target| {
        match event {
            Event::UserEvent(_) => {
                // As-written, this will only be called once. It is called
                // due to the `loop_proxy.send_event(())` call above.
                unsafe { add_url_handler() }
            }
            _ => {}
        }
    }).expect("TODO: panic message");
}

In order to use it, one would create a cargo project with the proper dependencies, drop the above source into main.rs, run cargo bundle, and modify the resulting app bundle's Info.plist per #2191 (comment). From there, dropping a document on the binary will result in calling the application delegate's application:openFiles: method, if defined.

[9mm]

@abhillman awesome! regarding your Info.plist, the only reason all the other keys are required if I remember right is that OSX requires that you explicitly specify every single filetype. Wildcard * does not work anymore (it's been awhile since I checked). I have confirmed in a number of other OSX apps and they all do this (specify every single file type), so I followed their lead.

@fredizzimo what do you think of that above? I don't really know rust well enough. That would be awesome if this works tho

Is there a way 2 people can collaborate on a PR so he can actually commit that to my PR?

[fredizzimo]

I don't really know what a good approach is, perhaps it would be worth asking the Winit team what their long-term vision is, and if this could be a good short-term workaround?

@9mm, regarding collaboration, you can either give the other person write permissions to your branch, or they can send a PR to your fork targeting your branch, which you can then merge. But you have to be careful with re-basing if you are co-operating on the same branch, it's the safest to always use merges, they will be cleaned up anyway when we finally squash the PR.

[polachok]

I sort of implemented this in polachok@9802ab3 with some limitations:

• it doesn't work when neovide isn't already open (not sure how to fix this)
• if multiple files are dragged onto icon, only one file is visible (macvim opens them in tabs)
• uses the WinitApplicationDelegate hack above

[9mm]

Awesome! I will check it out soon 💯

[polachok]

it doesn't work when neovide isn't already open (not sure how to fix this)

It actually works if I disable forking. Also, this makes "open with>Neovide" menu in finder work. I guess the "openFiles:" request is sent to the original process.

We should probably make no-fork the default if not running from tty, using atty crate. What do you think?

[9mm]

Interesting, thats def a question for @fredizzimo

Nice work this is super exciting. This is the main feature left thats painful, so Im really stoked!

[fredizzimo]

For the forking, we have this issue:

• Change the defaults for fork/no-fork #2147

TLDR;
I propose that we change the defaults to not fork, neither from the GUI nor from the terminal. That aligns better with how most applications behave when launched from the terminal. But it's not a decision I can take by my own.

[polachok]

Just opened a draft PR for this: #2395

[brnt]

regarding your Info.plist, the only reason all the other keys are required if I remember right is that OSX requires that you explicitly specify every single filetype. Wildcard * does not work anymore (it's been awhile since I checked). I have confirmed in a number of other OSX apps and they all do this (specify every single file type), so I followed their lead.

In my testing, the following addition to Info.plist seems to work. With it, all text-ish files now show neovide as an "Open with..." option.

<key>CFBundleDocumentTypes</key>
<array>
    <dict>
        <key>CFBundleTypeName</key>
        <string>All Text Files</string>
        <key>LSItemContentTypes</key>
        <array>
            <string>public.text</string>
        </array>
        <key>LSHandlerRank</key>
        <string>Owner</string>
    </dict>
</array>

I'm no expert, but ChatGPT says the public.text identifier declares that the application can open any file considered a text document.

[fredizzimo]

I guess this can be closed now?

[9mm]

I believe so!