sdk/io/file does not allow opening in append mode

RESOLVED INCOMPLETE

Status

RESOLVED INCOMPLETE
3 years ago
a year ago

People

(Reporter: erjoalgo, Assigned: erjoalgo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 Iceweasel/31.8.0
Build ID: 20150701023259

Steps to reproduce:

util/io is a thin wrapper around sdk/io/file

exports["test file-append-test"] = function(assert, done) {
    var io = require("../util/io.js");
    var io = require("util/io.js");
    // var io = require("io");//doesn't work
    var stringa = "hola",
	stringb = "adios",
	fn = "/tmp/jpm-file-test";
    
    io.writeTextToFile(fn, stringa);
    io.writeTextToFile(fn, stringb, "a");
    var contents = io.readTextFromFile(fn);
    var expected = stringa+stringb;
    assert.equal(contents, expected);
    done();
}


Actual results:

no append


Expected results:

append
You are redeclaring var io.

The first definition should be correct, assuming "util" is a sibling directory of the add-on's data directory.
(Assignee)

Comment 2

3 years ago
Sorry, that test is within my own addon, which defines its own io. I just included it to intuitively show what is wrong. 

I made a pull request here, tested it locally.

https://github.com/erjoalgo/addon-sdk/commit/dbe6d23bd727d06e54075a3d88a33ef8939f56b7
(Assignee)

Comment 3

3 years ago
Here's the fix that I'm using now which is the essence of the PR:

exports.open = function open(filename, mode) {
  var file = MozFile(filename);
  if (typeof(mode) !== "string")
    mode = "";

  // File opened for write only.
    if (/[aw]/.test(mode)) {
	var exists = file.exists();
    if (exists)
	ensureFile(file);
    var stream = Cc['@mozilla.org/network/file-output-stream;1'].
                 createInstance(Ci.nsIFileOutputStream);
      var openFlags = OPEN_FLAGS.WRONLY;
        
      if (exists && /a/.test(mode))	{
	  openFlags |= OPEN_FLAGS.APPEND;
      }else 	{
	  openFlags|= OPEN_FLAGS.TRUNCATE|
	      OPEN_FLAGS.CREATE_FILE;
      }

    var permFlags = parseInt("0644", 8); // u+rw go+r
    try {
      stream.init(file, openFlags, permFlags, 0);
    }
    catch (err) {
      throw friendlyError(err, filename);
    }
    return /b/.test(mode) ?
           new byteStreams.ByteWriter(stream) :
           new textStreams.TextWriter(stream);
  }

  // File opened for read only, the default.
  ensureFile(file);
  stream = Cc['@mozilla.org/network/file-input-stream;1'].
           createInstance(Ci.nsIFileInputStream);
  try {
    stream.init(file, OPEN_FLAGS.RDONLY, 0, 0);
  }
  catch (err) {
    throw friendlyError(err, filename);
  }
  return /b/.test(mode) ?
         new byteStreams.ByteReader(stream) :
         new textStreams.TextReader(stream);
};
(Assignee)

Comment 4

3 years ago
Created attachment 8667710 [details] [diff] [review]
Bug-1208898.patch patch for missing sdk/io/file.open append mode
Attachment #8667710 - Flags: review?(rFobic)
Attachment #8667710 - Flags: review?(dtownsend)
(Assignee)

Comment 5

3 years ago
added tests and squashed to one commit, as per the contributor instructions
(previous commit did not include tests, please disregard)
https://github.com/mozilla/addon-sdk/pull/2034
I assigned you the bug.
You can add a link to your pull request as an attachment to this bug and obsolete the previous attachments.
I can add commits to that same pull request should more changes be needed.
Assignee: nobody → erjoalgo
Oops, meant to say "You can add commits ..."
(Assignee)

Comment 8

3 years ago
It should be the same pull request:
https://github.com/mozilla/addon-sdk/pull/2034/

I fixed the formatting issues. Let me know if you notice anything else.
(Assignee)

Comment 9

3 years ago
Can someone take a look at this? I added a pull request last weeek and only got  Adrian Aichner [:anaran]'s code formatting comments, which I've since fixed.
Attachment #8667710 - Attachment is obsolete: true
Attachment #8667710 - Flags: review?(rFobic)
Attachment #8667710 - Flags: review?(dtownsend)
Created attachment 8670517 [details] [review]
pull request

Unfortunately I'd rather not take most of the style changes in this patch, I'd prefer to leave blame intact than follow the style guide to the letter. It looks like you should be able to implement this with just a couple of changes to choose between OPEN_FLAGS.TRUNCATE and OPEN_FLAGS.APPEND.

Re-request review on this when you've updated it, but don't worry about squashing the changes, I'll do that when it lands.
Attachment #8670517 - Flags: review-
https://bugzilla.mozilla.org/show_bug.cgi?id=1399562
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.