From 836719f72ed423ea828dcb6965dfd8291e69fb66 Mon Sep 17 00:00:00 2001 From: execjosh Date: Tue, 18 Dec 2012 15:23:26 +0900 Subject: [PATCH] Implement CommonJS IO/A read([n Number]) The [IO/A spec][1] for `read` is as follows: > Read up to n bytes from the stream, or until the end of the stream > has been reached. [If] n is null, reads up to the block size of the > underlying device, or up to 1024 bytes if the block size is not > discernible. If n is not specified, this method always reads the > full stream until its end is reached. ... Since discovering the block size of the underlying device is non-trivial, we will just default to 1024. **NOTE** The initial implementation of `File::read()` saves the current (original) position, seeks to the beginning of the stream, `readAll`s to the end, and then resets to the original position. I think that this behavior is unexpected and should be changed--it should read from the current position to the end of the stream and stay there. The user should explicitly `seek` to the beginning of the stream when necessary. With the current implementation, the user should note that the position *will not change* after calling `read()` with no arguments. [1]: http://wiki.commonjs.org/wiki/IO/A#Instance_Methods http://code.google.com/p/phantomjs/issues/detail?id=938 --- examples/stdin-stdout-stderr.coffee | 4 +-- examples/stdin-stdout-stderr.js | 4 +-- src/filesystem.cpp | 42 ++++++++++++++++++++++------- src/filesystem.h | 7 ++++- test/fs-spec-01.js | 11 ++++++++ 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/examples/stdin-stdout-stderr.coffee b/examples/stdin-stdout-stderr.coffee index 6bee88d8..60723e01 100644 --- a/examples/stdin-stdout-stderr.coffee +++ b/examples/stdin-stdout-stderr.coffee @@ -11,8 +11,8 @@ line = system.stdin.readLine() system.stdout.writeLine JSON.stringify line # This is essentially a `readAll` -system.stdout.writeLine 'system.stdin.read(): (ctrl+D to end)' -input = system.stdin.read() +system.stdout.writeLine 'system.stdin.read(5): (ctrl+D to end)' +input = system.stdin.read 5 system.stdout.writeLine JSON.stringify input phantom.exit 0 diff --git a/examples/stdin-stdout-stderr.js b/examples/stdin-stdout-stderr.js index 03734bed..80a43d38 100644 --- a/examples/stdin-stdout-stderr.js +++ b/examples/stdin-stdout-stderr.js @@ -11,8 +11,8 @@ var line = system.stdin.readLine(); system.stdout.writeLine(JSON.stringify(line)); // This is essentially a `readAll` -system.stdout.writeLine('system.stdin.read(): (ctrl+D to end)'); -var input = system.stdin.read(); +system.stdout.writeLine('system.stdin.read(5): (ctrl+D to end)'); +var input = system.stdin.read(5); system.stdout.writeLine(JSON.stringify(input)); phantom.exit(0); diff --git a/src/filesystem.cpp b/src/filesystem.cpp index 24a809ef..0b4f932e 100644 --- a/src/filesystem.cpp +++ b/src/filesystem.cpp @@ -59,8 +59,18 @@ File::~File() // encounters \0 or similar // public slots: -QString File::read() +QString File::read(const QVariant &n) { + // Default to 1024 (used when n is "null") + qint64 bytesToRead = 1024; + + // If parameter can be converted to a qint64, do so and use that value instead + if (n.canConvert(QVariant::LongLong)) { + bytesToRead = n.toLongLong(); + } + + const bool isReadAll = 0 > bytesToRead; + if ( !m_file->isReadable() ) { qDebug() << "File::read - " << "Couldn't read:" << m_file->fileName(); return QString(); @@ -71,17 +81,31 @@ QString File::read() } if ( m_fileStream ) { // text file - const qint64 pos = m_fileStream->pos(); - m_fileStream->seek(0); - const QString ret = m_fileStream->readAll(); - m_fileStream->seek(pos); + QString ret; + if (isReadAll) { + // This code, for some reason, reads the whole file from 0 to EOF, + // and then resets to the position the file was at prior to reading + const qint64 pos = m_fileStream->pos(); + m_fileStream->seek(0); + ret = m_fileStream->readAll(); + m_fileStream->seek(pos); + } else { + ret = m_fileStream->read(bytesToRead); + } return ret; } else { // binary file - const qint64 pos = m_file->pos(); - m_file->seek(0); - const QByteArray data = m_file->readAll(); - m_file->seek(pos); + QByteArray data; + if (isReadAll) { + // This code, for some reason, reads the whole file from 0 to EOF, + // and then resets to the position the file was at prior to reading + const qint64 pos = m_file->pos(); + m_file->seek(0); + data = m_file->readAll(); + m_file->seek(pos); + } else { + data = m_file->read(bytesToRead); + } QString ret(data.size()); for(int i = 0; i < data.size(); ++i) { ret[i] = data.at(i); diff --git a/src/filesystem.h b/src/filesystem.h index e61a13e3..aa4ded10 100644 --- a/src/filesystem.h +++ b/src/filesystem.h @@ -49,7 +49,12 @@ public: virtual ~File(); public slots: - QString read(); + /** + * @param n Number of bytes to read (a negative value means read up to EOF) + * NOTE: The use of QVariant here is necessary to catch JavaScript `null`. + * @see IO/A spec + */ + QString read(const QVariant &n = -1); bool write(const QString &data); bool seek(const qint64 pos); diff --git a/test/fs-spec-01.js b/test/fs-spec-01.js index 5eaab969..c759cd6a 100644 --- a/test/fs-spec-01.js +++ b/test/fs-spec-01.js @@ -37,6 +37,17 @@ describe("Basic Files API (read, write, remove, ...)", function() { expect(content).toEqual("hello\nworld\n"); }); + it("should be able to read specific number of bytes from a specific position in a file", function() { + var content = ""; + try{ + var f = fs.open(FILENAME, "r"); + f.seek(3); + content = f.read(5); + f.close(); + } catch (e) { } + expect(content).toEqual("lo\nwo"); + }); + it("should be able to read/write/append content from a file", function() { var content = ""; try{