Today
shurcooL commented on go1.9: net/http is broken on macOS … in github.com/gopherjs/gopherjs11h
This Go 1.9-specific issue has been resolved by e9fdce4974670cba5c74bd9c3e6308515ef8fcf3.
(The issue was just a place to post more information about what went wrong.)
This Go 1.9-specific issue has been resolved by e9fdce4974670cba5c74bd9c3e6308515ef8fcf3.
(The issue was just a place to post more information about what went wrong.)Yesterday
shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache12hAt this point, `n == 0` is rare enough, that the `if n > 0` check becomes unhelpful. Just omit it, this is still correct, but shorter and simpler:
```Go
func (r *cachingReadCloser) Read(p []byte)…
At this point, `n == 0` is rare enough, that the `if n > 0` check becomes unhelpful. Just omit it, this is still correct, but shorter and simpler:
```Go
func (r *cachingReadCloser) Read(p []byte)…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache12hActually, we can simplify this further.
`bytes.Buffer.Write` is documented to always return `nil` error, see https://godoc.org/bytes#Buffer.Write:
> The return value n is the length of p; err i…
Actually, we can simplify this further.
`bytes.Buffer.Write` is documented to always return `nil` error, see https://godoc.org/bytes#Buffer.Write:
> The return value n is the length of p; err i…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache12hShadowing can be very dangerous.
This line just introduced a very serious but subtle issue.
It's unfortunate to see that none of our tests caught this.
In my previous suggestion, I was caref…
Shadowing can be very dangerous.
This line just introduced a very serious but subtle issue.
It's unfortunate to see that none of our tests caught this.
In my previous suggestion, I was caref…shurcooL commented on It caused "dial tcp 127.0.0.1:8881:… in github.com/gopherjs/gopherjs13hMy advice was for using `net/http` in the browser, which provides XHR and Fetch APIs.
It doesn't work for node, because node doesn't provide those APIs. You'd need to install a polyfill. See #586.…
My advice was for using `net/http` in the browser, which provides XHR and Fetch APIs.
It doesn't work for node, because node doesn't provide those APIs. You'd need to install a polyfill. See #586.…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache15hAlso, drop the (unless len(p) is zero) part. It's not helpful.
If len(p) is zero, no data is requested, so there can't be no data to return since we didn't ask for any.
"If r has no data to ret…
Also, drop the (unless len(p) is zero) part. It's not helpful.
If len(p) is zero, no data is requested, so there can't be no data to return since we didn't ask for any.
"If r has no data to ret…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache15hWhat you're trying to do here is send a copy of what has been read into `r.buf` so far, in the form of an `io.ReadCloser` (or, as I suggested making it `io.Reader`).
I haven't done any benchmarks,…
What you're trying to do here is send a copy of what has been read into `r.buf` so far, in the form of an `io.ReadCloser` (or, as I suggested making it `io.Reader`).
I haven't done any benchmarks,…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache15hLet's the following change, I think it'll make this struct cleaner.
Change signature of `OnEOF` to `func(io.Reader)` here.
Let's the following change, I think it'll make this struct cleaner.
Change signature of `OnEOF` to `func(io.Reader)` here.shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache15hThis doesn't implement the `io.Reader` contract correctly. Its `Read` method documentation says:
> It returns the number of bytes read
However, what you're returning here is the number of byte…
This doesn't implement the `io.Reader` contract correctly. Its `Read` method documentation says:
> It returns the number of bytes read
However, what you're returning here is the number of byte…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache15hThis blank line isn't helpful, get rid of it (unless you think it's helpful, let me know).
This blank line isn't helpful, get rid of it (unless you think it's helpful, let me know).shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache16hs/when EOF is read/when EOF is reached/, for consistency with the doc string of `OnEOF`.
s/when EOF is read/when EOF is reached/, for consistency with the doc string of `OnEOF`.shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache17hActually, now that I wrote that up, that doesn't looks so great because the main logic ends up being on top... But I have a better idea. The `newCachedReadCloser` func, by now, offers very little val…
Actually, now that I wrote that up, that doesn't looks so great because the main logic ends up being on top... But I have a better idea. The `newCachedReadCloser` func, by now, offers very little val…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache17hI'd suggest `close(s.infChan)` or `close(s.done)` here. It does the same thing, but the intention would be more more clear IMO.
You're closing the channel (or sending a value on it) to signal that…
I'd suggest `close(s.infChan)` or `close(s.done)` here. It does the same thing, but the intention would be more more clear IMO.
You're closing the channel (or sending a value on it) to signal that…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache17hIt'd help to leave an inline comment explaining `infChan`, what it's for and what it's doing.
Also, from the way it's being used, can I suggest a better, more general name `done`?
```Go
done c…
It'd help to leave an inline comment explaining `infChan`, what it's for and what it's doing.
Also, from the way it's being used, can I suggest a better, more general name `done`?
```Go
done c…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache17hI think naked returns are bad for readability, I would suggest being explicit about what's being returned:
```Go
return n, err
```
I think naked returns are bad for readability, I would suggest being explicit about what's being returned:
```Go
return n, err
```shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache17hPersonally, I wouldn't be afraid of doing `resp := *resp`. I think its meaning should be well understood by Go programmers. See last code block of https://golang.org/doc/faq#closures_and_goroutines.
Personally, I wouldn't be afraid of doing `resp := *resp`. I think its meaning should be well understood by Go programmers. See last code block of https://golang.org/doc/faq#closures_and_goroutines.shurcooL commented on Shared context - texture gets black… in github.com/go-gl/glfw18hI'm on macOS, and I can't reproduce a black border.
I tried both places to load texture:
```Go
//texture = LoadTexture("square.png") // TEXTURE LOADS HERE WITHOUT AN ISSUE
window, err := glfw…
I'm on macOS, and I can't reproduce a black border.
I tried both places to load texture:
```Go
//texture = LoadTexture("square.png") // TEXTURE LOADS HERE WITHOUT AN ISSUE
window, err := glfw…shurcooL commented on Struct values not being cloned when… in github.com/gopherjs/gopherjs18hThanks for reporting, that looks like a legitimate bug related to JavaScript references vs Go values/pointers.
Thanks for reporting, that looks like a legitimate bug related to JavaScript references vs Go values/pointers.shurcooL commented on Do you have a tutorial or an exampl… in github.com/DHowett/go-plist18hThere is a simple use example in the [README](https://github.com/DHowett/go-plist/blob/master/README.md).
There are also 5 examples in [godoc](https://godoc.org/howett.net/plist#pkg-examples).
There is a simple use example in the [README](https://github.com/DHowett/go-plist/blob/master/README.md).
There are also 5 examples in [godoc](https://godoc.org/howett.net/plist#pkg-examples).shurcooL commented on "JavaScript error: ap.$get is not a… in github.com/gopherjs/gopherjs21hThanks for reporting, that looks like a bug in the type system. /cc @neelance
Thanks for reporting, that looks like a bug in the type system. /cc @neelanceshurcooL commented on It caused "dial tcp 127.0.0.1:8881:… in github.com/gopherjs/gopherjs21h> Does gopherjs support net/http module in golang ?
Yes, it does. I use `net/http` in all my frontend programs.
You need to use `http.DefaultTransport` rather than creating `&http.Transport{Max…
> Does gopherjs support net/http module in golang ?
Yes, it does. I use `net/http` in all my frontend programs.
You need to use `http.DefaultTransport` rather than creating `&http.Transport{Max…shurcooL commented on How to use go-gl with ANGLE? in github.com/go-gl/gl1dI don't have the answer to this question, because I'm not familiar with ANGLE and I don't use Windows as my primary OS at this time.
I would suggest trying to ask the question in other mediums, wh…
I don't have the answer to this question, because I'm not familiar with ANGLE and I don't use Windows as my primary OS at this time.
I would suggest trying to ask the question in other mediums, wh…This Week
shurcooL commented on Support preview Installations API -… in github.com/google/go-github1dI don't think we have those endpoints implemented at all. So, they need to be implemented.
We only have these two app/installation-related endpoints implemented:
- `GET /installation/repositori…
I don't think we have those endpoints implemented at all. So, they need to be implemented.
We only have these two app/installation-related endpoints implemented:
- `GET /installation/repositori…shurcooL commented on Big Numbers require a huge screen in github.com/robpike/ivy1d> Are there any demos for: Mac laptops, Windows.
Can you clarify what you mean by "demos", exactly?
ivy, as available in this repository, is just a command-line application. You can run it in a…
> Are there any demos for: Mac laptops, Windows.
Can you clarify what you mean by "demos", exactly?
ivy, as available in this repository, is just a command-line application. You can run it in a…shurcooL commented on Go 1.9 support. in github.com/gopherjs/gopherjs1dAs discussed, I'll run a short test for minification to cover it. Say, the `fmt` package:
```yml
- gopherjs test -v --minify fmt # Minification should work.
```
Done.
As discussed, I'll run a short test for minification to cover it. Say, the `fmt` package:
```yml
- gopherjs test -v --minify fmt # Minification should work.
```
Done.shurcooL commented on Go 1.9 support. in github.com/gopherjs/gopherjs1dI was a little worried about increasing the call stack by 1 more level in something so low-level, which is why I copied the implementation with a comment. But `runtime_SemacquireMutex` should be rare…
I was a little worried about increasing the call stack by 1 more level in something so low-level, which is why I copied the implementation with a comment. But `runtime_SemacquireMutex` should be rare…shurcooL commented on Go 1.9 support. in github.com/gopherjs/gopherjs1dDone in 8e80e53cc87099c2ae61f3aca9393b96ef955082.
Done in 8e80e53cc87099c2ae61f3aca9393b96ef955082.shurcooL commented on Support preview Installations API -… in github.com/google/go-github2d@S2606 I think you were added so you could be assigned to an issue (it's how GitHub permissions work). I'm not sure what you mean by "normal" procedure for PRs, but yes, you should just create a PR. …
@S2606 I think you were added so you could be assigned to an issue (it's how GitHub permissions work). I'm not sure what you mean by "normal" procedure for PRs, but yes, you should just create a PR. …shurcooL commented on add tokentype=basic to example code in github.com/google/go-github2dFriendly ping @queirozfcom. We won't be able to proceed with this PR until there's more information to help reproduce and understand this issue.
Friendly ping @queirozfcom. We won't be able to proceed with this PR until there's more information to help reproduce and understand this issue.
shurcooL commented on Remove custom media type for Merge … in github.com/google/go-github2dThis is resolved now. Thanks for the offer to help @RaviTezu! If you want to work on another issue, we still have many open, so feel free.
This is resolved now. Thanks for the offer to help @RaviTezu! If you want to work on another issue, we still have many open, so feel free.
shurcooL commented on delete custom media type 'applicati… in github.com/google/go-github2dThe simplifications were no longer applicable due to the changes that have since happened. I've rebased, resolved the merge conflict, improved the commit message, and updated the PR.
The simplifications were no longer applicable due to the changes that have since happened. I've rebased, resolved the merge conflict, improved the commit message, and updated the PR.
shurcooL commented on delete custom media type 'applicati… in github.com/google/go-github2dI'm going to apply the suggested simplifications myself.
I'm going to apply the suggested simplifications myself.shurcooL commented on Remove custom media type for Merge … in github.com/google/go-github2dI think we should just finish the existing PR and merge that. I'll do it now.
I think we should just finish the existing PR and merge that. I'll do it now.shurcooL commented on package dependencies must be instal… in github.com/go-gl/glfw2d@norbertfuhs, when you do `go get -u`, it installs the package you specified and all of its dependencies.
So you just need to reverse the order in which you run it. Use `go get` on the target pack…
@norbertfuhs, when you do `go get -u`, it installs the package you specified and all of its dependencies.
So you just need to reverse the order in which you run it. Use `go get` on the target pack…shurcooL commented on Use float for arcs in github.com/dominikh/go-js-dom2dThe [`CanvasRenderingContext2D.Rotate`](https://godoc.org/honnef.co/go/js/dom#CanvasRenderingContext2D.Rotate) method currently takes `int` parameter in `dom` package. It's documented to also use rad…
The [`CanvasRenderingContext2D.Rotate`](https://godoc.org/honnef.co/go/js/dom#CanvasRenderingContext2D.Rotate) method currently takes `int` parameter in `dom` package. It's documented to also use rad…shurcooL commented on Fix zip header name with wrong sep in github.com/mholt/archiver2dThis is significantly better than the original PR, thanks for adjusting!
LGTM but someone else more familiar with the zip spec needs to review. I'll leave it to @mholt or another collaborator.
This is significantly better than the original PR, thanks for adjusting!
LGTM but someone else more familiar with the zip spec needs to review. I'll leave it to @mholt or another collaborator.shurcooL commented on Fix zip header name with wrong sep in github.com/mholt/archiver2dOk, I think I understand now. You want to turn `name`, which is a filesystem path at this point, into a `/`-separated path.
If so, you should use `filepath.ToSlash` function for that. See its docu…
Ok, I think I understand now. You want to turn `name`, which is a filesystem path at this point, into a `/`-separated path.
If so, you should use `filepath.ToSlash` function for that. See its docu…shurcooL commented on Are error comparisons supported? in github.com/google/go-cmp2dSince it may be relevant, here's something I've used when I wanted to compare error (strings):
```Go
// equalError reports whether errors a and b are considered equal.
// They're equal if both a…
Since it may be relevant, here's something I've used when I wanted to compare error (strings):
```Go
// equalError reports whether errors a and b are considered equal.
// They're equal if both a…shurcooL commented on Fix zip header name with wrong sep in github.com/mholt/archiver2dAlso, there should be everything you need to manipulate paths in [`path/filepath`](https://godoc.org/path/filepath) and [`path`](https://godoc.org/path) packages. It shouldn't be neccessary to use `s…
Also, there should be everything you need to manipulate paths in [`path/filepath`](https://godoc.org/path/filepath) and [`path`](https://godoc.org/path) packages. It shouldn't be neccessary to use `s…shurcooL commented on Fix zip header name with wrong sep in github.com/mholt/archiver2d`filepath.Rel` returns `name` and `err`. It makes sense to check `err != nil` before proceeding.
So, you shouldn't insert the `name = ...` line here, it should be after `if err != nil { return err…
`filepath.Rel` returns `name` and `err`. It makes sense to check `err != nil` before proceeding.
So, you shouldn't insert the `name = ...` line here, it should be after `if err != nil { return err…shurcooL commented on Added function to check if GLFW is … in github.com/glfw/glfw2dSounds reasonable to me (although I always hope these things don't need to be added). But, of course, I defer to @elmindreda to know whether this change makes sense.
Sounds reasonable to me (although I always hope these things don't need to be added). But, of course, I defer to @elmindreda to know whether this change makes sense.shurcooL commented on Added function to check if GLFW is … in github.com/glfw/glfw2dThis change doesn't appear to be intentional.
This change doesn't appear to be intentional.shurcooL commented on Added function to check if GLFW is … in github.com/glfw/glfw2dI see. If there's no shared global state, is there a risk of race conditions if multiple places decide to check if glfw is initialized and call `glfwInit` if not?
I see. If there's no shared global state, is there a risk of race conditions if multiple places decide to check if glfw is initialized and call `glfwInit` if not?shurcooL commented on encoding/json: Unmarshal does not r… in github.com/golang/go2dI'm not sure if I see a bug here. It seems to me that `json.Unmarshal` is working as documented.
Have you read this part of the `encoding/json` docs?
> The Go visibility rules for struct fields…
I'm not sure if I see a bug here. It seems to me that `json.Unmarshal` is working as documented.
Have you read this part of the `encoding/json` docs?
> The Go visibility rules for struct fields…shurcooL commented on Added function to check if GLFW is … in github.com/glfw/glfw2dThe indentation is off. Existing comments use spaces, you've used tabs.
The indentation is off. Existing comments use spaces, you've used tabs.shurcooL commented on Added function to check if GLFW is … in github.com/glfw/glfw2d> to prevent repeated calls to `glfwInit()`
In what kind of situation would your program not know whether it already called `glfwInit` or not?
> to prevent repeated calls to `glfwInit()`
In what kind of situation would your program not know whether it already called `glfwInit` or not?shurcooL commented on Make constants typed in github.com/go-gl/glow2dI think only it's viable to put it in the same package if not using it means it gets dead code eliminated. Otherwise, we'd be increasing all production binary sizes.
I think only it's viable to put it in the same package if not using it means it gets dead code eliminated. Otherwise, we'd be increasing all production binary sizes.shurcooL commented on Panic free window destruction in github.com/go-gl/glfw2dDid you call `glfw.Init` at the beginning if your program, before calling `window.ShouldClose`?
`panic: NotInitialized: The GLFW library is not initialized` sounds like the kind of error you'd get…
Did you call `glfw.Init` at the beginning if your program, before calling `window.ShouldClose`?
`panic: NotInitialized: The GLFW library is not initialized` sounds like the kind of error you'd get…shurcooL commented on Remove custom media type for Merge … in github.com/google/go-github2dThat PR is 90% finished but he's not responding. I think we should just carry it ourselves.
That PR is 90% finished but he's not responding. I think we should just carry it ourselves.shurcooL commented on remove illegal `-r' ar option on Da… in github.com/google/woff23dPR title says `r`, but it's actually `f`.
The corresponding issue is #73.
PR title says `r`, but it's actually `f`.
The corresponding issue is #73.shurcooL commented on make clean all fails with error in github.com/google/woff23dI ran into this issue too, using the latest version of macOS and Xcode (Command Line Tools):
- macOS 10.12.6
- Xcode 8.3.3
I ran into this issue too, using the latest version of macOS and Xcode (Command Line Tools):
- macOS 10.12.6
- Xcode 8.3.3shurcooL commented on x/tools/cmd/goimports: package clau… in github.com/golang/go3dI think the expected output would be:
```Go
package main
import "fmt"
// a
// b
// c
func main() {
fmt.Println()
}
```
I think the expected output would be:
```Go
package main
import "fmt"
// a
// b
// c
func main() {
fmt.Println()
}
```shurcooL commented on examples directory should be rename… in github.com/gopherjs/vecty3dI think this applies to `docs` folder too, does it not?
See https://github.com/golang/go/tree/master/doc.
I think this applies to `docs` folder too, does it not?
See https://github.com/golang/go/tree/master/doc.Last Week
shurcooL commented on Add preview Topics support to more … in github.com/google/go-github5dThanks @RaviTezu!
Thanks @RaviTezu!
shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache5dAlso, given it's being assigned just `resp.Body` in `newCachedReadCloser`, or could we always just use `resp.Body`? If not, why not?
Also, given it's being assigned just `resp.Body` in `newCachedReadCloser`, or could we always just use `resp.Body`? If not, why not?shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache5dThis test is good, but have you considered also including the same test as in you provided in #70?
I think including that would ensure `httpcache` doesn't regress on its ability to support streami…
This test is good, but have you considered also including the same test as in you provided in #70?
I think including that would ensure `httpcache` doesn't regress on its ability to support streami…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache5dThis is odd. Why create a new `io.TeeReader` on each call to `cachedReadCloser.Read`?
Can you use `io.TeeReader` in `newCachedReadCloser` so it's only called once, returns a reader, and this `Read…
This is odd. Why create a new `io.TeeReader` on each call to `cachedReadCloser.Read`?
Can you use `io.TeeReader` in `newCachedReadCloser` so it's only called once, returns a reader, and this `Read…shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache5dIt's very common to give a variable like this a shorter name, like `buf`.
It's very common to give a variable like this a shorter name, like `buf`.shurcooL commented on Cache GET requests when body is ful… in github.com/gregjones/httpcache5dThis struct definitely needs to be documented. What is its purpose? What does it do? What are the requirements for using it correctly?
This struct definitely needs to be documented. What is its purpose? What does it do? What are the requirements for using it correctly?