Pluggable interfaces for import and export

RESOLVED FIXED

Status

RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: mvl, Assigned: mvl)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

import and export is currently done in one big (chrome) js file. This isn't
extendable. Also, the file isn't updated for the new api's.
(Assignee)

Comment 1

13 years ago
Created attachment 187285 [details] [diff] [review]
initial patch

This patch defines new interfaces and implements them. I didn't port all of the
importers/exporters. I doubt they are needed, but if they are, we can always
add them after we agree on an interface etc.

The importers/exporters are pluggable. nsICategoryManager is used to register
them
I don't like the way the extensions for the filepicker are stored in the
im/exporters. For example, *.htm and *.html will be two lines. That's not
really nice. But i do want to be able to have an extension that also exports to
html, but in some other cool layout, not the default boring list.

Comments on the interface or the impl?
(Assignee)

Updated

13 years ago
Attachment #187285 - Flags: first-review?(dmose)
(Assignee)

Updated

13 years ago
Blocks: 285016

Updated

13 years ago
Attachment #187285 - Flags: first-review?(dmose) → first-review?(pavlov)
(Assignee)

Comment 2

13 years ago
Created attachment 188229 [details] [diff] [review]
patch v2

Updated patch. Moved the files to better locations, remove the gCalendarWindow
stuff etc.
Attachment #187285 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #187285 - Flags: first-review?(pavlov)
(Assignee)

Updated

13 years ago
Attachment #188229 - Flags: first-review?(shaver)
Comment on attachment 188229 [details] [diff] [review]
patch v2

It's a shame we can't do arrays of AString, to avoid needing the calIFileType
interface...

I think we want the methods to be called importFromStream and exportToStream,
since we're not really exporting the stream.  (We sort of are importing the
stream, but I think the symmetry here will win.)

We should probably call the CSV importer "OutlookCSVImporter", since it's not
really a general CSV-handler, right?

spelling: "Comma Separated Values" (also  an l10n hazard, but that can be a
follow-up)

In parseTextField, you have an else after a return, which is a non-sequitur you
should avoid. =)

I don't see the calendar/Makefile.in change that would be needed to descend
into calendar/import-export and actually build all these goodies, which I
presume you want to do.

r=shaver with the above fixes made.  Sorry for the delay!
Attachment #188229 - Flags: first-review?(shaver) → first-review+
(Assignee)

Comment 4

13 years ago
Patch checked in. I will file new bugs for the l10n issues.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.