Re: Patch to support "F" fallback of XDS DND

[jdietrch fastmail fm]

On Mon, 15 Jun 2009 16:30 +0200, "Alexander Larsson" <alexl redhat com>
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 fastmail fm