Localizing exception messages

lzap · April 4, 2013, 11:08am

Hello,

I am working on extracting strings from the code base. Bryan already
started this effort and I am working on hard to finish it.

I have noticed that Bryan extracted strings from many exception
messages. Example:

raise _(&quot;Not implemented&quot;)

The issue I have is I saw we sometimes work with these messages.
Example:

rescue =&gt; e
  return case e.to_s
  when &quot;Must provide an operating systems&quot;
    _(&quot;Unable to find templates As this Host has no
        Operating System&quot;)
  else
    e.to_s
  end
end

(Try to grep for "case e" or "if e" to see more of these)

Now, I would like to discuss which approach should I commit to:

a) Translate all exception names except those which are being handled
programmatically. The issue I have with this option is this can be tough to
watch and there is a danger to miss few and bring some regressions in.

b) Do not translate any exception messages. The issue here is (I
believe) exceptions does get propagated into the UI so users would see
some messages in plain English.

c) Only mark them for translation and translate them programmatically
during rendering them to the screen. So all exceptions should look like
this:

raise N_(&quot;Not implemented&quot;)

That's how to mark a string for translation but not to actually
translate it. And then I would need to find the code where all the
exceptions are being put on a page where I would do:

_(exception_msg)

So it gets translated. I see this as the best option if there are just
few places where we can put the above snippet.

This should be relatively easy to check using a i18n-checker tool which
I am going to implement (will use Ivan Necas one from Katello). It
searches for _(…) uses in incorrect format (e.g. "unfriendly"
formatting of strings like "%s failed: %s" when translator does not know
what is what and is unable to swap them).

Opinions?

···

-- Later,

Lukas “lzap” Zapletal
#katello #systemengine

ohadlevy · April 4, 2013, 1:40pm

> Hello,
>
> I am working on extracting strings from the code base. Bryan already
> started this effort and I am working on hard to finish it.
>
> I have noticed that Bryan extracted strings from many exception
> messages. Example:
>
> raise _("Not implemented")
>
> The issue I have is I saw we sometimes work with these messages.
> Example:
>
> rescue => e
> return case e.to_s
> when "Must provide an operating systems"
> ("Unable to find templates As this Host has no
> Operating System")
> else
> e.to_s
> end
> end
>
> (Try to grep for "case e" or "if e" to see more of these)
>
> Now, I would like to discuss which approach should I commit to:
>
> a) Translate all exception names except those which are being handled
> programmatically. The issue I have with this option is this can be tough to
> watch and there is a danger to miss few and bring some regressions in.
>
> b) Do not translate any exception messages. The issue here is (I
> believe) exceptions does get propagated into the UI so users would see
> some messages in plain English.
>
> c) Only mark them for translation and translate them programmatically
> during rendering them to the screen. So all exceptions should look like
> this:
>
> raise N("Not implemented")
>
> That's how to mark a string for translation but not to actually
> translate it. And then I would need to find the code where all the
> exceptions are being put on a page where I would do:
>
> _(exception_msg)
>
> So it gets translated. I see this as the best option if there are just
> few places where we can put the above snippet.
>
> This should be relatively easy to check using a i18n-checker tool which
> I am going to implement (will use Ivan Necas one from Katello). It
> searches for _(…) uses in incorrect format (e.g. "unfriendly"
> formatting of strings like "%s failed: %s" when translator does not know
> what is what and is unable to swap them).
>
> Opinions?

What do you think about introducing error codes, and then we:

review all exceptions
make it easier to figure out the right error to translate?

Ohad

···

On Thu, Apr 4, 2013 at 2:08 PM, Lukas Zapletal wrote:

–
Later,

Lukas “lzap” Zapletal
#katello #systemengine

–
You received this message because you are subscribed to the Google Groups “foreman-dev” group.
To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Bryan_Kearney · April 4, 2013, 2:20pm

+1 to error codes, helps with hooking up to documentation.

– bk

···

On Thu, Apr 4, 2013 at 9:40 AM, Ohad Levy wrote:

On Thu, Apr 4, 2013 at 2:08 PM, Lukas Zapletal lzap@redhat.com wrote:
Hello,

I am working on extracting strings from the code base. Bryan already
started this effort and I am working on hard to finish it.

I have noticed that Bryan extracted strings from many exception
messages. Example:
raise _("Not implemented")
The issue I have is I saw we sometimes work with these messages.
Example:
rescue => e
  return case e.to_s
  when "Must provide an operating systems"
    _("Unable to find templates As this Host has no
        Operating System")
  else
    e.to_s
  end
end
(Try to grep for “case e” or “if e” to see more of these)

Now, I would like to discuss which approach should I commit to:

a) Translate all exception names except those which are being handled
programmatically. The issue I have with this option is this can be tough
to
watch and there is a danger to miss few and bring some regressions in.

b) Do not translate any exception messages. The issue here is (I
believe) exceptions does get propagated into the UI so users would see
some messages in plain English.

c) Only mark them for translation and translate them programmatically
during rendering them to the screen. So all exceptions should look like
this:
raise N_("Not implemented")
That’s how to mark a string for translation but not to actually
translate it. And then I would need to find the code where all the
exceptions are being put on a page where I would do:
_(exception_msg)
So it gets translated. I see this as the best option if there are just
few places where we can put the above snippet.

This should be relatively easy to check using a i18n-checker tool which
I am going to implement (will use Ivan Necas one from Katello). It
searches for _(…) uses in incorrect format (e.g. "unfriendly"
formatting of strings like “%s failed: %s” when translator does not know
what is what and is unable to swap them).

Opinions?
What do you think about introducing error codes, and then we:

review all exceptions

make it easier to figure out the right error to translate?

Ohad

–
Later,

Lukas “lzap” Zapletal
#katello #systemengine

–
You received this message because you are subscribed to the Google
Groups “foreman-dev” group.
To unsubscribe from this group and stop receiving emails from it, send
an email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

lzap · April 4, 2013, 2:02pm

What exactly do you have on your mind? Like doing something like:

raise "ERR4983 - user does not exist"

and then

if exception.message ~= /^ERR4983/

There is one more thing I need to keep in mind and this is exception
messages with parameters. We cannot use N_ function for these just like
that:

N_("fatal error: %s") % parameter

because this will not work (it will work but the message will not be
translated for obvious reasons). That's complication.

I am thinking about introducing new exception called ForemanException
which would accept message and all parameters and it would be
I18N-aware. Also we can extend it with error codes. Something like:

raise ForemanException.new("fatal error: %s", param)

or with a code

raise ForemanException.new(:ERR4983, "fatal error: %s", param)

then we can do things like

if exception == :ERR4983

The question is how folks would like much more longer raise statement. I
could provide a helper function for that, dsl-like one:

raise_code :ERR4983, "fatal error: %s", param1

raise_code :ERR4983, "fatal error: %{p1} %{p2}", :p1 => 42, :p2 => 15

But again not sure what is the benefit of error codes besides we can put
them in the documentation with some extra comments. Error codes brings
need for centralizing stuff which is not great (merge conflicts,
stealing numbers among pull requests etc).

···

On Thu, Apr 04, 2013 at 04:40:00PM +0300, Ohad Levy wrote: > What do you think about introducing error codes, and then we: > 1. review all exceptions > 2. make it easier to figure out the right error to translate?

–
Later,

Lukas “lzap” Zapletal
#katello #systemengine

Marek_Hulan · April 5, 2013, 6:24am

Hi,

I think that case construction based on exception message is just wrong.
Wouldn't be it worth of adding more exception classes so you would raise
Foreman::OperatingSystemMissing instead of checking exception message? I think
of exceptions messages as explanations of what and why it happened to a
programmer. Also some dynamic data can be part of exception message often (not
talking about Foreman necessarily e.g. "User #{name}" not allowed to do
#{something}"). To a user we should probably display a message formulated in
another way based only on exception type.

Error codes makes sense and could be part of those custom exceptions objects.
If they inherited from ForemanException as Lukas suggests it would work only
for Foreman exceptions so no error codes for system exceptions like
ArgumentError.

What do you think?

···

-- Marek

On Thursday 04 of April 2013 10:20:58 Bryan Kearney wrote:

+1 to error codes, helps with hooking up to documentation.

– bk

On Thu, Apr 4, 2013 at 9:40 AM, Ohad Levy ohadlevy@gmail.com wrote:
On Thu, Apr 4, 2013 at 2:08 PM, Lukas Zapletal lzap@redhat.com wrote:
Hello,

I am working on extracting strings from the code base. Bryan already
started this effort and I am working on hard to finish it.

I have noticed that Bryan extracted strings from many exception

messages. Example:
raise _(“Not implemented”)

The issue I have is I saw we sometimes work with these messages.

Example:
rescue => e
  return case e.to_s
  when "Must provide an operating systems"
  
    _("Unable to find templates As this Host has no
    
        Operating System")
  
  else
  
    e.to_s
  
  end

end
(Try to grep for “case e” or “if e” to see more of these)

Now, I would like to discuss which approach should I commit to:

a) Translate all exception names except those which are being handled
programmatically. The issue I have with this option is this can be tough
to

watch and there is a danger to miss few and bring some regressions in.

b) Do not translate any exception messages. The issue here is (I
believe) exceptions does get propagated into the UI so users would see
some messages in plain English.

c) Only mark them for translation and translate them programmatically
during rendering them to the screen. So all exceptions should look like

this:
raise N_(“Not implemented”)

That’s how to mark a string for translation but not to actually
translate it. And then I would need to find the code where all the

exceptions are being put on a page where I would do:
_(exception_msg)

So it gets translated. I see this as the best option if there are just
few places where we can put the above snippet.

This should be relatively easy to check using a i18n-checker tool which
I am going to implement (will use Ivan Necas one from Katello). It
searches for _(…) uses in incorrect format (e.g. "unfriendly"
formatting of strings like “%s failed: %s” when translator does not know
what is what and is unable to swap them).

Opinions?

What do you think about introducing error codes, and then we:

review all exceptions

make it easier to figure out the right error to translate?

Ohad

–
Later,

Lukas “lzap” Zapletal
#katello #systemengine

–
You received this message because you are subscribed to the Google

Groups “foreman-dev” group.

To unsubscribe from this group and stop receiving emails from it, send

an email to foreman-dev+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
–
Marek

lzap · April 5, 2013, 8:19am

Thanks for the input, I was under impression that string comparisons is
something that Ruby programmers do. This is I would say Java approach.

Threfore I am going to create superclass

class Foreman::Exception
def initialize message, *params
end

def error_code
"err" + hash(classname + message)
end

def message
"error_code: " + _(message) % {params}
end
end

For all exceptions we will need to catch or compare we will create
subclasses (no methods needs to be redefined if message is enough).

I will need to rewrite all raise statements to:

raise Foreman::Exception.new N_("message")

raise Foreman::Exception.new N_("message: %s"), param

raise Foreman::Exception.new N_("message: %{p1} %{p2}"), :p1 => 1, :p2 => 2

raise Foreman::Exception.new N_("message")

Maybe I can introduce some kind of helper like

raise_generic N_("message")

as a shortcut for Foreman::Exception, because it will be widely used
through the code base.

Plus we can build a small script that will go through the codebase
finding all the lines containing the raise and calculating error codes
for all possible error messages. So we can create a table into the
documentation for each release easily.

Opinions?

LZ

···

On Fri, Apr 05, 2013 at 08:24:54AM +0200, Marek Hulan wrote: > Hi, > > I think that case construction based on exception message is just wrong. > Wouldn't be it worth of adding more exception classes so you would raise > Foreman::OperatingSystemMissing instead of checking exception message? I think > of exceptions messages as explanations of what and why it happened to a > programmer. Also some dynamic data can be part of exception message often (not > talking about Foreman necessarily e.g. "User #{name}" not allowed to do > #{something}"). To a user we should probably display a message formulated in > another way based only on exception type. > > Error codes makes sense and could be part of those custom exceptions objects. > If they inherited from ForemanException as Lukas suggests it would work only > for Foreman exceptions so no error codes for system exceptions like > ArgumentError. > > What do you think? > > -- > Marek > > > On Thursday 04 of April 2013 10:20:58 Bryan Kearney wrote: > > +1 to error codes, helps with hooking up to documentation. > > > > -- bk > > > > On Thu, Apr 4, 2013 at 9:40 AM, Ohad Levy wrote: > > > On Thu, Apr 4, 2013 at 2:08 PM, Lukas Zapletal wrote: > > > > Hello, > > > > > > > > I am working on extracting strings from the code base. Bryan already > > > > started this effort and I am working on hard to finish it. > > > > > > > > I have noticed that Bryan extracted strings from many exception > > > > > > > > messages. Example: > > > > raise _("Not implemented") > > > > > > > > The issue I have is I saw we sometimes work with these messages. > > > > > > > > Example: > > > > rescue => e > > > > > > > > return case e.to_s > > > > when "Must provide an operating systems" > > > > > > > > _("Unable to find templates As this Host has no > > > > > > > > Operating System") > > > > > > > > else > > > > > > > > e.to_s > > > > > > > > end > > > > > > > > end > > > > > > > > (Try to grep for "case e" or "if e" to see more of these) > > > > > > > > Now, I would like to discuss which approach should I commit to: > > > > > > > > a) Translate all exception names except those which are being handled > > > > programmatically. The issue I have with this option is this can be tough > > > > > > to > > > > > > > watch and there is a danger to miss few and bring some regressions in. > > > > > > > > b) Do not translate any exception messages. The issue here is (I > > > > believe) exceptions does get propagated into the UI so users would see > > > > some messages in plain English. > > > > > > > > c) Only mark them for translation and translate them programmatically > > > > during rendering them to the screen. So all exceptions should look like > > > > > > > > this: > > > > raise N_("Not implemented") > > > > > > > > That's how to mark a string for translation but not to actually > > > > translate it. And then I would need to find the code where all the > > > > > > > > exceptions are being put on a page where I would do: > > > > _(exception_msg) > > > > > > > > So it gets translated. I see this as the best option if there are just > > > > few places where we can put the above snippet. > > > > > > > > This should be relatively easy to check using a i18n-checker tool which > > > > I am going to implement (will use Ivan Necas one from Katello). It > > > > searches for _(...) uses in incorrect format (e.g. "unfriendly" > > > > formatting of strings like "%s failed: %s" when translator does not know > > > > what is what and is unable to swap them). > > > > > > > > Opinions? > > > > > > What do you think about introducing error codes, and then we: > > > 1. review all exceptions > > > 2. make it easier to figure out the right error to translate? > > > > > > Ohad > > > > > > > -- > > > > Later, > > > > > > > > Lukas "lzap" Zapletal > > > > #katello #systemengine > > > > > > > > -- > > > > You received this message because you are subscribed to the Google > > > > > > Groups "foreman-dev" group. > > > > > > > To unsubscribe from this group and stop receiving emails from it, send > > > > > > an email to foreman-dev+unsubscribe@googlegroups.com. > > > > > > > For more options, visit https://groups.google.com/groups/opt_out. > > > > > > -- > > > You received this message because you are subscribed to the Google Groups > > > "foreman-dev" group. > > > To unsubscribe from this group and stop receiving emails from it, send an > > > email to foreman-dev+unsubscribe@googlegroups.com. > > > For more options, visit https://groups.google.com/groups/opt_out. > -- > Marek > > -- > You received this message because you are subscribed to the Google Groups "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out. > >

–
Later,

Lukas “lzap” Zapletal
#katello #systemengine