[release-branch.go1.4] code review 182560043: api: create go1.4.txt

api: create go1.4.txt

I read through and vetted these but others should look too.

[rsc, 2014-12-05 21:18:12]

Hello r (cc: adg, bradfitz, golang-codereviews@googlegroups.com, gri, iant),

I'd like you to review this change to the release-branch.go1.4 branch of
https://code.google.com/p/go/

[bradfitz, 2014-12-05 21:51:17]

I just read it.

One inconsistency that is kinda weird:

+# CL 122960043 encoding/xml: add InputOffset method to Decoder, Russ Cox <
rsc@golang.org>
+pkg encoding/xml, method (*Decoder) Input*Offset*() int64
+pkg encoding/xml, type SyntaxError struct, *Byte* int64

We already have: http://golang.org/pkg/encoding/json/#SyntaxError

package json
type SyntaxError struct {
        *Offset* int64 // error occurred after reading Offset bytes
        // contains filtered or unexported fields
}

But in Go 1.4, we the same thing, but named "Byte" instead of "Offset":

http://tip.golang.org/pkg/encoding/xml/#SyntaxError

type SyntaxError struct {
    Msg  string
    Line int
    *Byte* int64 // byte offset from start of stream
}

I suppose you could argue that xml.SyntaxError is different because it also
has Line, but I thought we didn't add that to json because the caller can
compute the Line.

I would like to rename xml's Byte to Offset.  (we can keep Line, or delete
it)


On Fri, Dec 5, 2014 at 1:18 PM, <rsc@golang.org> wrote:

> Reviewers: r,
>
> Message:
> Hello r (cc: adg, bradfitz, golang-codereviews@googlegroups.com, gri,
> iant),
>
> I'd like you to review this change to the release-branch.go1.4 branch of
> https://code.google.com/p/go/
>
>
> Description:
> api: create go1.4.txt
>
> I read through and vetted these but others should look too.
>
> Please review this at https://codereview.appspot.com/182560043/
>
> Affected files (+608, -143 lines):
>   A api/go1.4.txt
>   M api/next.txt
>   M src/cmd/api/run.go
>
>
>

[bradfitz, 2014-12-05 21:53:31]

Oh, and as highlighted but not mentioned: the xml package isn't even
internally consistent, naming it InputOffset in one place (also new) and
Byte in the other. Naming it Offset would be internally consistent and
across json+xml.


On Fri, Dec 5, 2014 at 1:51 PM, Brad Fitzpatrick <bradfitz@golang.org>
wrote:

> I just read it.
>
> One inconsistency that is kinda weird:
>
> +# CL 122960043 encoding/xml: add InputOffset method to Decoder, Russ Cox <
> rsc@golang.org>
> +pkg encoding/xml, method (*Decoder) Input*Offset*() int64
> +pkg encoding/xml, type SyntaxError struct, *Byte* int64
>
> We already have: http://golang.org/pkg/encoding/json/#SyntaxError
>
> package json
> type SyntaxError struct {
>         *Offset* int64 // error occurred after reading Offset bytes
>         // contains filtered or unexported fields
> }
>
> But in Go 1.4, we the same thing, but named "Byte" instead of "Offset":
>
> http://tip.golang.org/pkg/encoding/xml/#SyntaxError
>
> type SyntaxError struct {
>     Msg  string
>     Line int
>     *Byte* int64 // byte offset from start of stream
> }
>
> I suppose you could argue that xml.SyntaxError is different because it
> also has Line, but I thought we didn't add that to json because the caller
> can compute the Line.
>
> I would like to rename xml's Byte to Offset.  (we can keep Line, or delete
> it)
>
>
> On Fri, Dec 5, 2014 at 1:18 PM, <rsc@golang.org> wrote:
>
>> Reviewers: r,
>>
>> Message:
>> Hello r (cc: adg, bradfitz, golang-codereviews@googlegroups.com, gri,
>> iant),
>>
>> I'd like you to review this change to the release-branch.go1.4 branch of
>> https://code.google.com/p/go/
>>
>>
>> Description:
>> api: create go1.4.txt
>>
>> I read through and vetted these but others should look too.
>>
>> Please review this at https://codereview.appspot.com/182560043/
>>
>> Affected files (+608, -143 lines):
>>   A api/go1.4.txt
>>   M api/next.txt
>>   M src/cmd/api/run.go
>>
>>
>>
>

[rsc, 2014-12-05 21:54:00]

I'll change Byte -> Offset. Thanks.

[minux, 2014-12-07 02:25:37]

https://codereview.appspot.com/182560043/diff/60001/api/go1.4.txt
File api/go1.4.txt (right):

https://codereview.appspot.com/182560043/diff/60001/api/go1.4.txt#newcode476
api/go1.4.txt:476: pkg syscall (windows-386), func LoadCreateSymbolicLink()
error
Why is this needed? Why not just make CreateSymbolicLink return an error if
it's not supported on the current platform?

[bradfitz, 2014-12-07 23:58:26]

LGTM

[adg1, 2014-12-08 00:05:10]

LGTM

[adg1, 2014-12-08 00:06:33]

I'll submit this shortly.

[adg, 2014-12-08 00:30:20]

No, I won't.​

[rsc, 2014-12-10 00:06:10]

https://codereview.appspot.com/182560043/diff/60001/api/go1.4.txt
File api/go1.4.txt (right):

https://codereview.appspot.com/182560043/diff/60001/api/go1.4.txt#newcode476
api/go1.4.txt:476: pkg syscall (windows-386), func LoadCreateSymbolicLink()
error
On 2014/12/07 02:25:37, minux wrote:
> Why is this needed? Why not just make CreateSymbolicLink return an error if
> it's not supported on the current platform?

I don't know, but it's too late to rewrite that code. I don't mind this being in
pkg syscall. It's a drop in the bucket.

[adg, 2014-12-10 00:07:49]

*** Submitted as https://code.google.com/p/go/source/detail?r=82920819eae8 ***

[release-branch.go1.4] api: create go1.4.txt

I read through and vetted these but others should look too.

LGTM=bradfitz, adg
R=r, minux, bradfitz, adg
CC=adg, golang-codereviews, gri, iant
https://codereview.appspot.com/182560043

Committer: Andrew Gerrand <adg@golang.org>

[brainman, 2014-12-10 03:19:57]

On 2014/12/07 02:25:37, minux wrote:
> api/go1.4.txt:476: pkg syscall (windows-386), func LoadCreateSymbolicLink()
> error
> Why is this needed? ...

I assume you know the answer to that question.

> ... Why not just make CreateSymbolicLink return an error if
> it's not supported on the current platform?

What about if you want to know if CreateSymbolicLink is supported on your
system, but you don't want to call CreateSymbolicLink?

Alex

[minux, 2014-12-10 03:28:55]

On Tue, Dec 9, 2014 at 10:19 PM, <alex.brainman@gmail.com> wrote:

> ... Why not just make CreateSymbolicLink return an error if
>> it's not supported on the current platform?
>>
>
> What about if you want to know if CreateSymbolicLink is supported on
> your system, but you don't want to call CreateSymbolicLink?
>
why do you need to learn that? If you want to create a symlink, then just
create it,
and if that fails, use a fallback.

And if you really want to learn if symlink is supported, you can always
load kernel32.dll
and look for it.

Or put it another way, should we introduce LoadXXX for every function XXXX
that
we want to use but might not be available on every version of windows?

[minux, 2014-12-10 03:35:01]

On Tue, Dec 9, 2014 at 10:28 PM, minux <minux@golang.org> wrote:
>
> Or put it another way, should we introduce LoadXXX for every function XXXX
> that
> we want to use but might not be available on every version of windows?
>
OK. It seems syscall package for windows has already established the
convention
that for every function that is not universally available, add a Load
function for that.

I should have complained much earlier. That's not how the syscall package
for other
systems works. (they will return ENOSYS if the syscall doesn't exist. And
only getwd
is special cased to have an ImplementsGetwd const.)

I withdraw my objection.