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.

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

Add Text File Importer #952

Merged
merged 32 commits into from
Nov 1, 2018
Merged

Add Text File Importer #952

merged 32 commits into from
Nov 1, 2018

Conversation

roundhill
Copy link
Contributor

@roundhill roundhill commented Oct 24, 2018

Building on #922 with an importer for text files in a folder. You simply construct a TextFileImporter with the note and tag bucket, and then pass along an array of File objects to importNotes():

const textImporter = new TextFileImporter({
        this.props.noteBucket,
        this.props.tagBucket
      });

      textImporter.on('status', (eventType, message) => {
        console.log(eventType, message);
      });

      const filesArray = [array of `File` objects]
      textImporter.importNotes(filesArray);

It will only pull in files that end in .txt that are less than 1mb in size.

Refs #916

@roundhill roundhill added this to the 1.2.2 milestone Oct 24, 2018
@roundhill roundhill requested a review from mirka October 24, 2018 19:16
…browser 👍

The class now expects an array of `File`s to be passed into it.
for (const file of filesArray) {
importTextFile(file);
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we emit a 'complete' status event at the end of this, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is async so I need to find a way to know when we've reached the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a solution in 2bdc4bf. I'm not in love with it since it may fire early due to the async nature of FileReader, but it will probably suffice.

Copy link
Member

@mirka mirka Oct 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw this is exactly where Promises come in handy ✨

Simplified example:

const importTextFile = file => {
  return new Promise((resolve, reject) => {
    const fileReader = new FileReader();
    fileReader.onload = event => {

      // import note here...

      this.emit('status', 'progress', importedNoteCount);
      resolve();
    };

    fileReader.readAsText(file);
  });
};

Promise.all(filesArray.map(importTextFile))
  .then(() => {
    this.emit('status', 'complete', importedNoteCount);
  });

(Not a blocker, the lastFileName strategy is probably good enough and we can promisify it later!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice! I haven't worked with Promises before. 👍

@mirka mirka mentioned this pull request Oct 27, 2018
3 tasks
@roundhill
Copy link
Contributor Author

roundhill commented Oct 29, 2018

I think we may want to see if the title of the file is contained in the text. I have an export from apple notes that puts the title of the note in the file title, but not in the content:

😘Breakfast.txt

If the title isn't the first part of the content, we should use it as the title.

@roundhill
Copy link
Contributor Author

If the title isn't the first part of the content, we should use it as the title.

Added via 876c7be

@roundhill roundhill changed the base branch from feature/import-parser to master November 1, 2018 18:34
@roundhill roundhill merged commit 6be0efa into master Nov 1, 2018
@roundhill roundhill deleted the feature/text-file-parser branch November 1, 2018 18:45
@SeanKilleen
Copy link

@roundhill thanks so much for this! Is going to save me a ton of time in my life.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants