World: r4wp

Brief restatement: BROWSE isn't working on OSX -- the current implementation 
tries two system() calls: xdg-open and x-www-browser

This patch uses /usr/bin/open on OSX, maintains the previous handlers 
on other platforms. In addition, I replaced the system() calls with 
fork+exec to avoid firing up another shell; one byproduct of this 
is that exec errors no longer print to the console (though this could 
have been hackishly solved by adding some redirects to the system() 
call string in the existing codebase).

Did you test this on Linux as well?

Not yet, can do so now.

Please do.

Firing up the VM, give me 5 mins.

No hurries :)

Look here http://issue.cc/r3/1921before you do this. Make sure the 
security constraints mentioned there are taken into account.

(Works on my Fedora instance.)

Thanks for testing that.

Basically: looks fine, thanks a lot for working on this.

BrianH: I don't think this change is an ideal solution, but it seems 
to be an improvement on prior behaviour.

If you can find a way to be more specific on OSX, so that _only_ 
URLs get handled, that would be great. OTOH, xdg-open is already 
rather versatile as well (only I fear that OSX's open is even more 
featureful).

One problem with the existing implementation is that xdg-open and 
x-www-browser are searched for in the PATH, so executables with the 
same names in a PATH directory that takes precedence will yield different 
behaviour.

Andreas: yeah, file:/// will open in Finder with this pathc.

patch*.

(So it's not constrained to the browser.)

That's the same with xdg-open.

Right.

Yeah. It looks like it just calls open, which can be a bit of a hole. 
It's better for now but we need to come up with a better solution 
in the long run.

Agreed.

Hmm, why does BROWSE accept NONE?

BrianH: I don't mean to present this patch as something it's not. 
To clarify: still a hacky solution, but at least it brings the current 
behaviour to OS X.

No, agreed. It's still nowhere near as bad as the #1921 approach, 
or the last ticket that requested it. But we need to make sure to 
come up with something better in the long run.

For sure.

I might patch this NONE case unless anyone has a good reason to leave 
it in place?

(At the moment, you get usage info on stderr. The current r3 just 
segfaults on Linux.)

It's none propagation I guess. Because dealing with or ignoring nones 
is easier to do in Rebol than writing conditional code around everything 
to avoid triggering errors.

Okay, so I guess we just need to handle the NONE rather than attempting 
to browse it

The BROWSE code should explicitly do nothing if passed a NONE. It 
certainly shouldn't segfault.

If it doesn't explicitly do nothing, that's a bug.

Just return unset, I guess.

Okay, fixing now.

not R_NONE?

No, that returns none. Whatever BROWSE normally returns.

It normally returns NONE.

R_NONE

But I think the original intent was for NONE to open an empty browser. 
Can't find any references for that.

Weird. Shouldn't it return unset?

https://github.com/rebol/r3/blob/master/src/core/n-io.c#L499

Should probably rather return the URL, that would be more useful 
:)

That treatment of a none argument makes sense, Andreas. However, 
the function should return unset, like PRINT.

If we can agree on something, I'll push that the change.

BrianH: So, R_UNSET?

That's independent of the OSX open fix in any case.

Sure. And in the has-a-url case too.

So better just file a CC issue for the `browse none` crash, and we 
can discuss the desired design in there.

Yeah, maybe two pulls.

Okay, I'll pop this in a separate branch.

Marc, did you file a ticket for this? If not, I can repurpose #1921 
for this.