On Sat, 2005-01-01 at 07:46 -0800, Kees Cook wrote:

On Fri, Dec 31, 2004 at 05:29:27PM -0400, bulia byak wrote:

...

(BTW, will someone please fix the default-plain-svg-on-save bug? It's SO annoying.)

The order of the file extensions comes from extension/db.cpp

Problem seems to be that the internal extension list is stored as a map (moduledict). This does not retain the extension initialization order, unfortunately. Why this just started, though, I'm not sure. It looks like that code wasn't changed. I've added a list now to track init order. This seems to fix things. However, the "open" file list is a little weird too, even prior to my changes. I've tried to re-order things so that the GDK image loader comes absolutely last.

What's the "right" way to handle file extension order?

However, why was glob handling removed? Did it get implemented somewhere else? (cvs rev 1.16 for db.cpp)

Well, this was kinda broken when we switched to the new file dialog. The new file dialog doesn't save what format was used last in the preferences (like the previous one did) -- so basically the first one is selected.

Now, what aggravated this is that I went and cleaned up some of the code which grabs lists from the extensions database, so the order changed. So, just randomly, it switched from Inkscape SVG to Plain SVG.

1st fix, need to update the new file selectors to use preferences.

2nd, I'm not sure which order that the list should be in. Probably alphabetical -- but I'm not sure. This also comes down to what order should all the lists of modules be in?

--Ted

PS - I see that Kees you changed it to keep the same order as initialized, which isn't bad. It would make it so that the internal modules would end up first, thoughts anyone?

Currently, opening a raster image seems to be not working again. Is it maybe not fetching the right extension? Maybe this is unrelated, or a coincidence.

Kees Cook wrote:

On Mon, Jan 03, 2005 at 12:29:17AM -0700, Ted Gould wrote:

...

2nd, I'm not sure which order that the list should be in. Probably alphabetical -- but I'm not sure. This also comes down to what order should all the lists of modules be in?

PS - I see that Kees you changed it to keep the same order as initialized, which isn't bad. It would make it so that the internal modules would end up first, thoughts anyone?

Yeah, this is basically the question I asked too. I'm not sure what's "right", but I was trying to compare the "before" list with the "after" list and it seemed like the extensions were mostly in init order, so that's what I implemented.

I think alphabetical (by filename extension, not description) should be the way to go. With the GDK stuff in there, the list is giant already, so navigating it should be made easier. It should not be alphabetical until the last-saved pref is restored, though. Dunno. It should be "intuitive"... but I'm not sure what that really means for an open/save dialog. :)

On Mon, 2005-01-03 at 07:51 -0600, Bob Jamison wrote:

Currently, opening a raster image seems to be not working again. Is it maybe not fetching the right extension? Maybe this is unrelated, or a coincidence.

Works for me... What format were you using? I was doing PNGs.

--Ted

Bob Jamison wrote:

Well, I did a cvs update and make clean;make just to be certain, but images are not loading, and the Open dialog shows nothing with the "All images" file filter selected.

I hope it's not just me. ;-)

Bob

FYI:

I have this as a test at filedialog.cpp, line 779:

Inkscape::Extension::db.get_input_list(extension_list);

g_message("db.get_input_list: %d\n", extension_list.size());

...and it prints 0

Bob

Bob Jamison wrote:

Ted & All,

I did a lot of searching/debugging/tracing & stuff, and it all came down to line 50 in db.cpp:

bool add_to_list=((*moduledict.find(module->get_id())).second == NULL);

EEEEEEEEEEEeeeeeeeeeeeeeeeeeeeeeeeeeeeeeekkkkkkkkkk!!!!!!!!

Scary, scary, scary code!

That's got a few of my pet-peeves, mainly the nestling of calls there. Sure, it saves a few lines in the source, but we're not working from punch-cards or teletypes any more.

Among other things, collapsing the get_id() and find() calls make it hard to debug. In general I recommend storing the results of get_id(), storing the result of find() and *then* checking second. Trust me, it does nothing to make the code run faster. :-)

Kees Cook wrote:

On Mon, Jan 03, 2005 at 12:45:36PM -0800, Jon A. Cruz wrote:

...

Scary, scary, scary code!

Yeah, my bad. I don't know stl very well yet.

Well, not really. Or at least not your 'bad'. That's just a very common coding practice.

Way too common for my tastes, but then again I can be known to get pedantic and/or opinionated now and then.

:-)

...

Among other things, collapsing the get_id() and find() calls make it hard to debug. In general I recommend storing the results of get_id(), storing the result of find() and *then* checking second. Trust me, it does nothing to make the code run faster. :-)

Can you go in there and fix it up the way it "should" be, and I can look at it when you're done?

I'll try to get to it this week. I'll also check against our coding style guides and make sure things are properly in there also. That's the main point about griping publically - gives some record and mention for helping others avoid subtle pitfalls.