Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

go/token: FileSet.AddFile function signature complaint #2499

Closed
gopherbot opened this issue Nov 26, 2011 · 5 comments
Closed

go/token: FileSet.AddFile function signature complaint #2499

gopherbot opened this issue Nov 26, 2011 · 5 comments
Labels
FrozenDueToAge

Comments

@gopherbot
Copy link

by raul.san@sent.com:

With the next signature and its comment, isn't very clear (at least for me) that the
source could be something different than a file (like a string or []byte)

    func (s *FileSet) AddFile(filename string, base, size int) *File

+ As suggestion, could be used "*FileSet.AddFile" to get it from a file, in
addition of "*FileSet.AddString". So, there is no confussion.

+ The argument size is not necessary since it should calculated by that function (saving
code when you use that function). For the file:

    file, err := os.Open(filename)
    if err != nil {
        return err
    }
    info, err := file.Stat()
    if err != nil {
        return err
    }

    size := info.Size
    file.Close()


http://golang.org/pkg/go/token/#FileSet.AddFile
@adg
Copy link
Contributor

adg commented Nov 27, 2011

Comment 1:

Owner changed to @griesemer.

@rsc
Copy link
Contributor

rsc commented Nov 28, 2011

Comment 2:

I am not sure there is anything to change.
You do have to learn that File means a File object and
not an operating system file.  That is true for both
the go/token and go/ast packages.  After that, this is fine.

@griesemer
Copy link
Contributor

Comment 3:

This is working as intended.
I agree that it would be better if one wouldn't have to specify the size of the "file" -
not so much because of this specific API but because it would permit clients of
token.Positions to work on streams (io.Reader) of input where the length might not be
know a priori.
A different implementation may make that possible. Closing this for now.

Status changed to WorkingAsIntended.

@gopherbot
Copy link
Author

Comment 4 by raul.san@sent.com:

At least, it could be renamed the argument "filename" to "name".

@griesemer
Copy link
Contributor

Comment 5:

It could, but this data structure is a about "File Sets", so filename is an appropriate
name. Also, note that a "file" doesn't have to be something stored on a disk.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rsc @adg @griesemer @gopherbot