[haiku-commits] Re: r34140 - haiku/trunk/src/apps/screenshot

  • From: Fredrik Modèen <fredrik@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 20 Nov 2009 11:26:58 +0100 (CET)

> Fredrik Modèen <fredrik@xxxxxxxxx> wrote:
>> >> It did work, or if I moved to the define before I tested it, don't
>> >> remember.
>> > But you know that translators aren't fixed, right? You can add and
>> > remove translators as you choose, so while this solution would have
>> > worked in your particular setup (or a clean Haiku image), it's
>> > still a
>> > pretty much brain dead solution. Sorry for being harsh, but I'm
>> > really
>> > a bit appalled.
>> Yes I know. I Didn't thought about how to handle if a translator
>> didn't
>> exist. I thought those things was already taken care of in the
>> original
>> code, but perhaps one should,'t taken things for given :)
>
> Not sure if you got the point yet: the translator ID is something chosen
> at runtime - it will differ if the translators are different. IOW what
> has ID '8' on your system could have ID '4' on mine.
> I think you should really just follow what "translate" is doing - this
> is completely generic, and you are able to choose translators the app
> writer didn't think of before.
I thought I did, but now I do. If translate are generic perhaps i class or
somthing that more apps can use would be good?

anyway I will have a look at translate.cpp and ask if I have questions ;)

>
>> and now I know a new word "appalled"
>
> Always look on the bright side ;-)
are there any other way? :)

>
>> Perhaps a error message should be shown stating that a translator for
>> the
>> given format don't exist before trying to make a ScreenshotWindow?
>
> I think if you are using command line options, no window should be
> shown ever, anyway. The whole point of command line options are to be
> able to use the command from a script. And does normally simply exclude
> any GUI.
No to a question :)

to make the command line option not create a Window object should I
extract the things needed for making command line option work and make the
ordinary Window option use the same as command line option?

To clarify.
Move _TakeScreenshot(), _SaveScreenshot() and things around it to the App
class or perhaps it's own class?

>
> Bye,
>    Axel.
>
>
>


-- 
MVH
Fredrik Modèen


Other related posts: