Leave strerror here, as you're using it.

I wouldn't make this the same exception type as other errors. It will only happen when loading the package, I can be a standard ErrorException. Ideally, BinDeps would provide a standardize way of checking this, with a specific error.

I don't see the point of this GeneralError type. The idea behind using specific exception types was precisely that they can be adapted to the specific information you need to convey. In my message to julia-users, I simplified the typology a bit, but the other constants from above should all be replaced with an exception type.

The point of a GeneralError type was to be able (in the future) to have the actual messages table driven, based on the locale, so a French locale could display error strings in French, for example.

Translating error messages is another big issue that should be tackled in Julia more broadly. Let's keep that out of scope for now.

Right, but this does nothing more than what UnicodeError already does in Base, to allow for the format strings to be constant - which is what allows the error handling to be done without forcing the entire function to always push a GC frame (I know that Yuyichao has talked about changing things so that that is not so bad, by only doing that along branches that do the creating, but I'm not sure what the status of the in v0.4.x or master).
Yes, the whole locale business is something to think about for the future, but I think having GeneralError which greatly simplifies making the specific exception types, is well worth having here.

I think it would be better to have all StringEncodingError subtypes contain those fields instead, and get rid of GeneraError. That way, you can make args a tuple of the right length for each type.

But wouldn't that mean adding a lot of code to have specific show methods for every single type?

No, because you should be able to write a generic function for StringEncodingError.

Hmm, OK, I'll try to figure out how to do that.

OK, wasn't as painful as I thought it would be, but does mean instead of a single value for each subtype of StringEncodingError, they need to copy the three that GeneralError had.
Let me know what you think of this commit, thanks!

Sorry, this looks good, but I've come to think that instead of storing a msg field which would be identical for all instances; it would be better to have message(::Type{InvalidEncodingError}), etc., functions which will return the message template. Then it should be OK.

That should also allow you to get rid of inner constructors (or of most of them).

Hehe, no problem at all, sounds like a good idea! Just hope you are OK with all the time on this, thanks!

As I've been changing it to use message, I had a thought, that maybe this loses some important flexibility. If you use constructors, it you can have different messages, based on the number or types of the arguments to the constructors, but with this scheme, how would you manage that?
For this particular package, at least now, it probably doesn't matter, but I still am thinking about what we'd want for a much more flexible and efficient general error scheme for Julia.

You could still define the method to return the msg field if it's actually there for the considered type. So I don't think we'd lose in generality at all.

Another issue, is that I think it adds a dynamic dispatch when you actually do call show, whereas before, that was not necessary.

Ok, I'll try that.

I tried a slightly different method that looked a bit cleaner - let me know what you think, I added a subtype for StringEncodingErrors that don't take any arguments, makes the show() method for thos just a simple one-liner.

Rather than an abstract type, I'd prefer if you used a type union for show. Also put the two show methods at the same place for clarity. That way, the type tree won't reflect implementation details: subtypes(StringEncodingError) will only return interesting information for the user.

After that, we should be done!

OK, done!

Again, no need to use the longer parametric function syntax here, just put the Union after the argument (also, putting the function on two lines would be better).

OK, that's just how I've usually seen it in the code in Base - will change.

It actually turned out to be a few characters longer - before I had message(T), now it is message(typeof(exc)), but that's fine.

As you wish, but it would be nice to make the line shorter by splitting it.

InvalidSequenceError would be more precise and consistent with the other one.

The reason this was separate, was that this one didn't have the byte sequence available.
Do you have a good solution for that?

Ah, I see. Then just remove that branch, as I don't think it can happen (the call doesn't pass any input data...). Anyway we'll get a generic error if it happens.