remove _global_windows state, add security-token, local file URI helpers #14
Conversation
Electron seems to not allow relative URLs. I suppose it is not sure what they are relative to? Handling them properly is a bit verbose (electron/electron#11560), so I expect it will be helpful to have a standardized interface provided.
| @@ -4,46 +4,59 @@ module Electron | |||
| using JSON, URIParser | |||
|
|
|||
| export Application, Window, URI, windows, applications | |||
| export @LOCAL | |||
There was a problem hiding this comment.
I think I would prefer to not have this in here, but instead just add support for https://github.com/rofinn/FilePaths.jl to this package here.
There was a problem hiding this comment.
I think he's previously stated that he doesn't want this in there (rofinn/FilePaths.jl#18)
But this also doesn't have any analogous functionality to what's provided by that package. This macro is a way of building a URI to a local file relative to the current source file. That package is a way of representing a path to a local file (a slightly more limited concept) relative to the cwd.
| connection::IO | ||
| proc | ||
| sysnotify_connection::IO | ||
| secure_cookie::Vector{UInt8} | ||
| windows::Vector{T} |
There was a problem hiding this comment.
The only reason for T here is that we don't have access to Window yet, right? Or is there some other reason?
There was a problem hiding this comment.
Yeah, it's a forward declaration
| main_pipe_name = generate_pipe_name("juliaelectron-$process_id-$id") | ||
| ispath(main_pipe_name) || break | ||
| id += 1 | ||
| end |
There was a problem hiding this comment.
Maybe an even less involved way to do this would be to just generate a UUID as the id and use that? Then we don't have to track anything, and also not check whether the pipe already exists?
There was a problem hiding this comment.
nah, this is easier, since you generally want to handle collisions anyways. this is basically how tempfile usually is often implemented (although typically now they start from a random number to reduce the search)
| # filespec isa String && return URI_file(base, filespec) # can construct eagerly | ||
| # return :(URI_file($base, $(esc(filespec)))) | ||
| return :(URI_file(@__DIR__, $(esc(filespec)))) | ||
| end |
There was a problem hiding this comment.
I think ideally we would just put something like that into FilePaths.jl, right? That would seem like a better home.
There was a problem hiding this comment.
I'm hoping my PR to URIParser will be accepted, and then can later drop this compat code from here. I use this to operate on full URI file:/ objects, not just the subset covered by FilePaths, so it wouldn't fit into the later.
|
I'll merge this for now, but will unexport |
More changes. 🙂
If you need
windows(), it can still be defined asfoldl(append!, Window[], (windows(app) for app in Electron._global_applications)). Do you want me to add that?Also, this adds a set of helper function for working with local file URI objects (similar to loadFile that was recently added to Electron in electron/electron#11560)