Patch to support "F" fallback of XDS DND

[jdietrch]

As I recently added XDS support to the application I am developing,
I needed a file manager that also supported XDS. It was a bit
surprising to discover that nautilus doesn't currently implement
the full XDS standard (http://www.newplanetsoftware.com/xds/).
In particular, nautilus doesn't support the "F" fallback of XDS,
which allows the file manager to receive the data for the dropped
file via the X server and save the file. This is important to me,
because my application is run on a remote machine, and cannot save
files to the local machine. It depends on the "F" fallback to get
the file saved.

So I patched nautilus to fully support XDS and opened this bug
report with a full description. I have also attached the patch and
a test program. See http://bugzilla.gnome.org/show_bug.cgi?id=585790

Since I use Debian, I first reported this to the Debian bug tracking
system. But the package maintainer suggested that it would be better
to discuss this directly with upstream. So that's what I'm doing.
I'd love to see this feature included in the upstream source. It would
certainly be useful to me, as well as to anyone else who uses XDS with
an app running on a remote machine.

Questions? Suggestions? Anything else I can do to help?

Thank you for your time,
James Dietrich
--
nautilus-list mailing list
nautilus-list@...
http://mail.gnome.org/mailman/listinfo/nautilus-list

[Alexander Larsson]

On Sun, 2009-06-14 at 19:38 -0400, James Dietrich wrote:

> As I recently added XDS support to the application I am developing,
> I needed a file manager that also supported XDS. It was a bit
> surprising to discover that nautilus doesn't currently implement
> the full XDS standard (http://www.newplanetsoftware.com/xds/).
> In particular, nautilus doesn't support the "F" fallback of XDS,
> which allows the file manager to receive the data for the dropped
> file via the X server and save the file. This is important to me,
> because my application is run on a remote machine, and cannot save
> files to the local machine. It depends on the "F" fallback to get
> the file saved.
>
> So I patched nautilus to fully support XDS and opened this bug
> report with a full description. I have also attached the patch and
> a test program. See http://bugzilla.gnome.org/show_bug.cgi?id=585790
>
> Since I use Debian, I first reported this to the Debian bug tracking
> system. But the package maintainer suggested that it would be better
> to discuss this directly with upstream. So that's what I'm doing.
> I'd love to see this feature included in the upstream source. It would
> certainly be useful to me, as well as to anyone else who uses XDS with
> an app running on a remote machine.
>
> Questions? Suggestions? Anything else I can do to help?

I'm not sure why you changed the APIs to use GString. I don't think that
really helps, it just means we have to duplicate the data in various
places. I'd prefer to add a length argument to the functions you made
accept a GString.

Also, it seems like we're allocating memory for the whole file before
saving. Thats gonna be kinda bad for large files. Also, do we really
pass all that data as a single x message?

Minor nits: There is a bunch of missing spaces before parenthesis in
function calls.


--
nautilus-list mailing list
nautilus-list@...
http://mail.gnome.org/mailman/listinfo/nautilus-list

[jdietrch]

On Mon, Jun 15, 2009 at 04:30:08PM +0200, Alexander Larsson wrote:

> I'm not sure why you changed the APIs to use GString. I don't think that
> really helps, it just means we have to duplicate the data in various
> places. I'd prefer to add a length argument to the functions you made
> accept a GString.
>
> Also, it seems like we're allocating memory for the whole file before
> saving. Thats gonna be kinda bad for large files. Also, do we really
> pass all that data as a single x message?
>
> Minor nits: There is a bunch of missing spaces before parenthesis in
> function calls.
>

Thank you for taking the time to evaluate the patch. These are some
good questions and suggestions. I'll do my best to address them and
then come back with an improved patch.

James Dietrich
--
nautilus-list mailing list
nautilus-list@...
http://mail.gnome.org/mailman/listinfo/nautilus-list

[jdietrch]

On Mon, 15 Jun 2009 16:30 +0200, "Alexander Larsson" <alexl@...>
wrote:

> On Sun, 2009-06-14 at 19:38 -0400, James Dietrich wrote:
> > As I recently added XDS support to the application I am developing,
> > I needed a file manager that also supported XDS. It was a bit
> > surprising to discover that nautilus doesn't currently implement
> > the full XDS standard (http://www.newplanetsoftware.com/xds/).
> > In particular, nautilus doesn't support the "F" fallback of XDS,
> > which allows the file manager to receive the data for the dropped
> > file via the X server and save the file. This is important to me,
> > because my application is run on a remote machine, and cannot save
> > files to the local machine. It depends on the "F" fallback to get
> > the file saved.
> >
> > So I patched nautilus to fully support XDS and opened this bug
> > report with a full description. I have also attached the patch and
> > a test program. See http://bugzilla.gnome.org/show_bug.cgi?id=585790
> >
> > Since I use Debian, I first reported this to the Debian bug tracking
> > system. But the package maintainer suggested that it would be better
> > to discuss this directly with upstream. So that's what I'm doing.
> > I'd love to see this feature included in the upstream source. It would
> > certainly be useful to me, as well as to anyone else who uses XDS with
> > an app running on a remote machine.
> >
> > Questions? Suggestions? Anything else I can do to help?
>

Thank you for the feedback. I've finally taken the time to work on this
again, and have attached a new patch to the report in bugzilla:
http://bugzilla.gnome.org/show_bug.cgi?id=585790
The new patch applies cleanly to nautilus 2.28.1 and 2.29.1.

> I'm not sure why you changed the APIs to use GString. I don't think that
> really helps, it just means we have to duplicate the data in various
> places. I'd prefer to add a length argument to the functions you made
> accept a GString.

I have done as requested and added a length argument instead of using
a GString.

>
> Also, it seems like we're allocating memory for the whole file before
> saving. Thats gonna be kinda bad for large files. Also, do we really
> pass all that data as a single x message?

Yes, we do allocate memory for the whole file before saving. But
nautilus
already does this same thing when it receives a text drop, though of
course
a text drop is likely to be much smaller than a binary file. So it's
definitely a valid point, but perhaps it's one that would be better
solved
in a separate patch and by someone more familiar with nautilus than I
am.

As for whether the data is passed as a single x message, I don't know.
What I did find out is that ICCCM describes an INCR property for sending
and receiving x selections incrementally.
See http://tronche.com/gui/x/icccm/sec-2.html#s-2.7.2
However, I am not sure if GTK implements this or not. Perhaps someone
more knowledgeable than I could comment on that.

I did test this patch with a 235 MB file, as well as several smaller
ones,
and it worked just fine.

>
> Minor nits: There is a bunch of missing spaces before parenthesis in
> function calls.

These should be all gone now.

I appreciate your taking the time to look at this again, and look
forward
to any feedback.

Thanks,
James Dietrich
--
 
  jdietrch@...

--
nautilus-list mailing list
nautilus-list@...
http://mail.gnome.org/mailman/listinfo/nautilus-list

[Alexander Larsson]

On Tue, 2009-10-27 at 20:08 -0400, jdietrch@... wrote:

> On Mon, 15 Jun 2009 16:30 +0200, "Alexander Larsson" <alexl@...>
> wrote:
> > On Sun, 2009-06-14 at 19:38 -0400, James Dietrich wrote:
> > > As I recently added XDS support to the application I am developing,
> > > I needed a file manager that also supported XDS. It was a bit
> > > surprising to discover that nautilus doesn't currently implement
> > > the full XDS standard (http://www.newplanetsoftware.com/xds/).
> > > In particular, nautilus doesn't support the "F" fallback of XDS,
> > > which allows the file manager to receive the data for the dropped
> > > file via the X server and save the file. This is important to me,
> > > because my application is run on a remote machine, and cannot save
> > > files to the local machine. It depends on the "F" fallback to get
> > > the file saved.
> > >
> > > So I patched nautilus to fully support XDS and opened this bug
> > > report with a full description. I have also attached the patch and
> > > a test program. See http://bugzilla.gnome.org/show_bug.cgi?id=585790
> > >
> > > Since I use Debian, I first reported this to the Debian bug tracking
> > > system. But the package maintainer suggested that it would be better
> > > to discuss this directly with upstream. So that's what I'm doing.
> > > I'd love to see this feature included in the upstream source. It would
> > > certainly be useful to me, as well as to anyone else who uses XDS with
> > > an app running on a remote machine.
> > >
> > > Questions? Suggestions? Anything else I can do to help?
> >
>
> Thank you for the feedback. I've finally taken the time to work on this
> again, and have attached a new patch to the report in bugzilla:
> http://bugzilla.gnome.org/show_bug.cgi?id=585790
> The new patch applies cleanly to nautilus 2.28.1 and 2.29.1.

Commited to master. Thanks.

Btw, when testing with dnd.py i didn't get a filename with the raw
version. Is that a bug or does dnd.py just don't supply one?


--
nautilus-list mailing list
nautilus-list@...
http://mail.gnome.org/mailman/listinfo/nautilus-list

[jdietrch]

On Tue, Nov 17, 2009 at 01:13:40PM +0100, Alexander Larsson wrote:

> On Tue, 2009-10-27 at 20:08 -0400, jdietrch@... wrote:
> > On Mon, 15 Jun 2009 16:30 +0200, "Alexander Larsson" <alexl@...>
> > wrote:
> > > On Sun, 2009-06-14 at 19:38 -0400, James Dietrich wrote:
> > > > As I recently added XDS support to the application I am developing,
> > > > I needed a file manager that also supported XDS. It was a bit
> > > > surprising to discover that nautilus doesn't currently implement
> > > > the full XDS standard (http://www.newplanetsoftware.com/xds/).
> > > > In particular, nautilus doesn't support the "F" fallback of XDS,
> > > > which allows the file manager to receive the data for the dropped
> > > > file via the X server and save the file. This is important to me,
> > > > because my application is run on a remote machine, and cannot save
> > > > files to the local machine. It depends on the "F" fallback to get
> > > > the file saved.
> > > >
> > > > So I patched nautilus to fully support XDS and opened this bug
> > > > report with a full description. I have also attached the patch and
> > > > a test program. See http://bugzilla.gnome.org/show_bug.cgi?id=585790
> > > >
> > > > Since I use Debian, I first reported this to the Debian bug tracking
> > > > system. But the package maintainer suggested that it would be better
> > > > to discuss this directly with upstream. So that's what I'm doing.
> > > > I'd love to see this feature included in the upstream source. It would
> > > > certainly be useful to me, as well as to anyone else who uses XDS with
> > > > an app running on a remote machine.
> > > >
> > > > Questions? Suggestions? Anything else I can do to help?
> > >
> >
> > Thank you for the feedback. I've finally taken the time to work on this
> > again, and have attached a new patch to the report in bugzilla:
> > http://bugzilla.gnome.org/show_bug.cgi?id=585790
> > The new patch applies cleanly to nautilus 2.28.1 and 2.29.1.
>
> Commited to master. Thanks.
>
> Btw, when testing with dnd.py i didn't get a filename with the raw
> version. Is that a bug or does dnd.py just don't supply one?

Only the XDS version can supply a filename, so it is expected that the
raw version doesn't supply one. In that case a filename is supplied by
nautilus, and I hard-coded it to be "dropped data". Is that what you
were seeing, or was there not any filename at all?

James
--
nautilus-list mailing list
nautilus-list@...
http://mail.gnome.org/mailman/listinfo/nautilus-list

[Alexander Larsson]

On Wed, 2009-11-18 at 20:09 -0500, James Dietrich wrote:

> On Tue, Nov 17, 2009 at 01:13:40PM +0100, Alexander Larsson wrote:
> > On Tue, 2009-10-27 at 20:08 -0400, jdietrch@... wrote:
> > > On Mon, 15 Jun 2009 16:30 +0200, "Alexander Larsson" <alexl@...>
> > > wrote:
> > > > On Sun, 2009-06-14 at 19:38 -0400, James Dietrich wrote:
> > > > > As I recently added XDS support to the application I am developing,
> > > > > I needed a file manager that also supported XDS. It was a bit
> > > > > surprising to discover that nautilus doesn't currently implement
> > > > > the full XDS standard (http://www.newplanetsoftware.com/xds/).
> > > > > In particular, nautilus doesn't support the "F" fallback of XDS,
> > > > > which allows the file manager to receive the data for the dropped
> > > > > file via the X server and save the file. This is important to me,
> > > > > because my application is run on a remote machine, and cannot save
> > > > > files to the local machine. It depends on the "F" fallback to get
> > > > > the file saved.
> > > > >
> > > > > So I patched nautilus to fully support XDS and opened this bug
> > > > > report with a full description. I have also attached the patch and
> > > > > a test program. See http://bugzilla.gnome.org/show_bug.cgi?id=585790
> > > > >
> > > > > Since I use Debian, I first reported this to the Debian bug tracking
> > > > > system. But the package maintainer suggested that it would be better
> > > > > to discuss this directly with upstream. So that's what I'm doing.
> > > > > I'd love to see this feature included in the upstream source. It would
> > > > > certainly be useful to me, as well as to anyone else who uses XDS with
> > > > > an app running on a remote machine.
> > > > >
> > > > > Questions? Suggestions? Anything else I can do to help?
> > > >
> > >
> > > Thank you for the feedback. I've finally taken the time to work on this
> > > again, and have attached a new patch to the report in bugzilla:
> > > http://bugzilla.gnome.org/show_bug.cgi?id=585790
> > > The new patch applies cleanly to nautilus 2.28.1 and 2.29.1.
> >
> > Commited to master. Thanks.
> >
> > Btw, when testing with dnd.py i didn't get a filename with the raw
> > version. Is that a bug or does dnd.py just don't supply one?
>
> Only the XDS version can supply a filename, so it is expected that the
> raw version doesn't supply one. In that case a filename is supplied by
> nautilus, and I hard-coded it to be "dropped data". Is that what you
> were seeing, or was there not any filename at all?

Ah. Yeah, thats what i was seeing. I was assuming there was some way to
hint a filename.

--
nautilus-list mailing list
nautilus-list@...
http://mail.gnome.org/mailman/listinfo/nautilus-list