Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Manually add \r to Windows phrases pre 1.4.5 #3615

Merged
merged 3 commits into from Nov 25, 2016
Merged

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Nov 25, 2016

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. B0-patch labels Nov 25, 2016
@jacogr jacogr changed the title Manually add \r to Windows phrases pre 1.4.4 Manually add \r to Windows phrases pre 1.4.5 Nov 25, 2016
if (createType === 'fromPhrase' && windowsPhrase) {
phrase = phrase
.split(' ') // get the words
.map((w) => ['zoom', 'misjudged'].includes(w) ? w : `${w}\r`) // add \r after each (except last in dicts)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So I guess we see two words here because one is coming from 1.4 dict, and the second one is in 1.5 dict.

  1. We need to check if that's actually the case (that there is no new line character after last word)
  2. The word from 1.4 dict cannot be present in 1.5 dict for this code to work.

I think it would be better to only support 1.4 (so only one word), master branch is known to be unstable, and we should just mention that in the release notes instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tomusdrw
Copy link
Collaborator

Would be good to test it on windows (especially the misjudged case) when it's in

@ngotchac
Copy link
Contributor

Looks good. Would be great to test on windows, as @tomusdrw said.

@ngotchac ngotchac added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 25, 2016
@ngotchac ngotchac merged commit 5058576 into master Nov 25, 2016
@ngotchac ngotchac deleted the jg-windows-phrase branch November 25, 2016 17:43
@3esmit
Copy link

3esmit commented Nov 25, 2016

If I created the wallet in linux; and try to import it's recorvery phrase in windows, this wouldn't make a new issue?

Edit: checked the source and saw that a checkbox was added for informing that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants