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
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.
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.
Peace, James
------------------------------------------------------------------------------
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
