BoostIO / BoostNote-Legacy

This repository is outdated and new Boost Note app is available! We've launched a new Boost Note app which supports real-time collaborative writing. https://github.com/BoostIO/BoostNote-App
Other
17.07k stars 1.47k forks source link

Note clone slash to backslash #2310

Open tsuyoshicho opened 6 years ago

tsuyoshicho commented 6 years ago

Current behavior

Image from Gyazo

Cloned note are slash to backslash converted.(ex #2018\08\20)

Is this specification?

Expected behavior

Clone note are same.

Steps to reproduce

  1. Create note.(note ex. first line #2018/08/20)
  2. Clone note.
  3. Check cloned note body.(result ex #2018\08\20)

Environment

zzxxhhzxh commented 6 years ago

In 0.11.8, windows 10 home, it occurs too. It seems that all the "/" will be replaced by "\" when cloning.

simosdev commented 6 years ago

Checked this with current master (fdfa3bb). Seems to be problem with Object.assign in browser/main/lib/dataApi/createNote.js createNote(). Added console.logs to trace where the content changes, title does not seem to change.

function createNote (storageKey, input) {
  console.log('createNote() input at start of function', input);
  let targetStorage
  try {
    if (input == null) throw new Error('No input found.')
    input = Object.assign({}, input)
    validateInput(input)

    targetStorage = findStorage(storageKey)
  } catch (e) {
    return Promise.reject(e)
  }

  return resolveStorageData(targetStorage)
    .then(function checkFolderExists (storage) {
      if (_.find(storage.folders, {key: input.folder}) == null) {
        throw new Error('Target folder doesn\'t exist.')
      }
      return storage
    })
    .then(function saveNote (storage) {
      let key = keygen(true)
      let isUnique = false
      while (!isUnique) {
        try {
          sander.statSync(path.join(storage.path, 'notes', key + '.cson'))
          key = keygen(true)
        } catch (err) {
          if (err.code === 'ENOENT') {
            isUnique = true
          } else {
            throw err
          }
        }
      }
      console.log('createNote() input before Object.assign()', input);
      const noteData = Object.assign({},
        {
          createdAt: new Date(),
          updatedAt: new Date()
        },
        input, // input may contain more accurate dates
        {
          key,
          storage: storageKey
        })

      console.log('createNote() noteData after Object.assign()', noteData);
      CSON.writeFileSync(path.join(storage.path, 'notes', key + '.cson'), _.omit(noteData, ['key', 'storage']))

      return noteData
    })
}

image

gbirman commented 6 years ago

On the other end of the spectrum, backslashes ("\") are being converted to forward slashes ("/") on macOS Sierra in cloned/moved files with Boostnote 0.11.9.

urda commented 6 years ago

This impacted me on 0.11.9, I'm shocked this isn't higher priority it's not OK if my note data changes just from a clone operation.

nagledb commented 5 years ago

Further oddity: the backslashes go back to their original form if you close and reopen Boostnote without editing the note.

Steps:

  1. Create a note with content containing slashes, such as example / and \\.
  2. Clone the note.
  3. View cloned note and observe that the slashes have modified (to example / and // in my case, on Mac OS X).
  4. Quit Boostnote without modifying the note.
  5. Reopen Boostnote.
  6. View the cloned note and observe that the now matches the original note (i.e., example / and \\).

However if you modify the cloned note before quitting Boostnote (between steps 3 and 4), the slashed get fixed in their modified state.

jdmota commented 5 years ago

@simosdev You should be careful when console logging objects, because the object might be changed between the time you log and the time you "expand" the object on the console.

I believe this is the code that changes the note's contents:

image

image

More precisely, this line:

https://github.com/BoostIO/Boostnote/blob/cfc84f3e78369297a6772159c0fe300262a929b6/browser/main/lib/dataApi/attachmentManagement.js#L373

simosdev commented 5 years ago

@jdmota Thanks. Didn't really make sense that it would've been broken on Object.assign(). Made too much assumptions it would be broken on createNote() as I did not use any attachments during testing.

This explains why the behavior is different in Windows vs Linux/Mac. The flipping forward or backslash reverting after restarting an unmodified clone also makes sense now. browser/main/NoteList/index.js cloneNote() first creates note and saves it to disk, after that attachmentManagement.cloneAttachments() is called which bugs out the note, but does not save to disk. This also causes a bug where a cloned unmodified note will have the original note's attachment identifier when Boostnote is restarted.