On Wed, Dec 1, 2010 at 3:13 PM, James Bowlin <bow...@mindspring.com> wrote:
> On Wed, Dec 01, 2010 at 01:52 PM, Grissiom said:
>> Nice work ;)
>
> Thanks.
>
>> I have a suggestion regard with BROWSER. I think we should
>> check BROWSER at first. If it fails for some reason(invalid
>> name, dead link etc.), we should warn the user and proceed to
>> find a suitable browser by our own. I think mix the BROWSER and
>> {graphical,text}_browsers are bad idea because users won't be
>> aware of the wrong settings.
>
> Originally help.fish would blindly use $BROWSER if the variable
> existed regardless of whether it it existed or was appropriate
> (it tried to open firefox in a vconsole and I got no help).
>
> By mixing $BROWSER in with the lists, I was able to accomplish
> most of the things you are requesting, plus other good things:
>
> 1) Test $BROWSER before blindly assuming it exists.
>
> 2) Fallback to our list of browsers if $BROWSER fails.
>
> 3) Don't try $BROWSER first in text mode unless we know
> it is a text browser.
>
> 4) But use it as a last resort in text mode as long as
> it is not a known graphical browser.
>
> The only thing I don't do is warn the user if $BROWSER didn't
> work. This is slightly complicated by the fact that we don't
> try $BROWSER first in text mode (unless it is a known text
> browser). But something like this might work:
>
> if test "$BROWSER" = "$browsers[1]" -a "$BROWSER" ~= "$fish_browser"
> echo "Error message"
> end
>
Hmm, seems cool. But I think that means we would check BROWSER twice. That's
not a very good deal ;) I want to write a demo code but I found
function in fish could
NOT receive parameters!(WTF!) So I give up right now...
> If it was the first choice and we didn't select it then it must
> be broken. I should probably loop over $CONSOLE_BROWSER and
> $BROWSER (yet again).
>
>
>> P.S. it seems you use mixed tab and space indent... fish_indent
>> is your friend ;)
>
> Thanks. Sorry about that. I had been using:
>
>>XX> functions help > ~/fish.help
>
> to create the version I upload. I figured that would do the right
> thing. I'll try to pipe it through my poorly patched version of
> fish_indent next time.
>
>> P.P.S. the if block started in line 101 seems a bit complected, is
>> that what you mean? ;)
>
> The comment says:
> #------------------------------------------------------------------------
> # Always try user's browser first in graphical mode. Only try it first
> # in text mode if it is a known text browser, otherwise use it as a last
> # resort, if it is not a known graphical browser, since by then we have
> # little to lose.
> #------------------------------------------------------------------------
>
> $graphical_browsers contains the list of browser we will check in
> graphical mode. $text_browsers contains the list we will check in
> text mode. So we always prepend $BROWSER to $graphical_browsers,
> we prepend it to $text_browsers if it is a known text browser,
> otherwise, we append it to $text_browsers as long as it is not
> a known graphical browser.
>
> I can use the "not" builtin to get rid of the else clause,
> perhaps this is what you were hinting at.
>
Yup ;) I think if we would have a "elseif" keyword the code will be cleaner.
(A lot of work to do, Yeah? ;)
> I think I'm doing the right thing in that block and I think I'm
> doing it as simply as possible (modulo the minor change above).
> But if that's the case then I need to add some comments because
> the existing code and comments are still confusing.
>
> IMO the block starting on line 101 was the heart of my changes
> to the code. If I've won you over to my was of seeing things,
> can you offer a suggestion of what comments to add to make this
> more clear for others? Perhaps I could add the 4-point list
> above to the existing comment.
>
I think your 4-points list is clear enough to convince me ;) Nice work.
Thank you ;)
--
Cheers,
Grissiom
------------------------------------------------------------------------------
Increase Visibility of Your 3D Game App & Earn a Chance To Win $500!
Tap into the largest installed PC base & get more eyes on your game by
optimizing for Intel(R) Graphics Technology. Get started today with the
Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev
_______________________________________________
Fish-users mailing list
Fish-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/fish-users
