Conversion shortcuts to Typescript

[pushkov-fedor]

Fixes #519

[pushkov-fedor]

I used git mv, but i'm not sure that i used it in correctly. Is everything okay with commits?
I've found some issues with cursor shortcuts after conversion, they don't work in some cases and behavior is different on Chrome and Mac, which is strange. Need a little time to check it and fix.

[raineorshine]

I used git mv, but i'm not sure that i used it in correctly. Is everything okay with commits?

You can see on the "Files Changed" tab that many files have been deleted and recreated rather than renamed. You'll need to fix this so that they are renamed, otherwise we lose continuity of the git history. I would try an interactive rebase where you can reset the deleted file and try again. I have had issues with renames before, so it's not uncommon. Maybe you can search online and find a proper explanation and solution.

I've found some issues with cursor shortcuts after conversion, they don't work in some cases and behavior is different on Chrome and Mac, which is strange. Need a little time to check it and fix.

Okay, thanks

[pushkov-fedor]

I used git mv, but i'm not sure that i used it in correctly. Is everything okay with commits?

You can see on the "Files Changed" tab that many files have been deleted and recreated rather than renamed. You'll need to fix this so that they are renamed, otherwise we lose continuity of the git history. I would try an interactive rebase where you can reset the deleted file and try again. I have had issues with renames before, so it's not uncommon. Maybe you can search online and find a proper explanation and solution.

Okay, i will google it

[raineorshine]

Could you provide an update? We need to get this merged. Thanks!

[pushkov-fedor]

Could you provide an update? We need to get this merged. Thanks!

Yes, sorry, i was a little loaded with other issues. I will provide changes today or tomorrow!

[pushkov-fedor]

Raine, hi! I've tested all files, but i'm not confident with bindContext (i can't figure out what this feature does, i tried invoking alt + shift + b on a Thoughh and then adding children to Thought's context or to Thought itself, but nothing changes), cursorForward (it did nothing for me on commits before conversion and after), deleteEmptyThoughtOrOutdent (in case when we have

• parent
  • emptyThought
    • emptyThought

setting cursor on parent/<emptyThought> and pressing Backspacedo nothing (before conversion and after).

I think we need to add tests for cursor and newThought.../newSubthoughts shortcuts in order to be confident that everything works. After conversion code passed all tests, but some issues were found out in testing all shortcuts by hand.

Also i haven't succeed in renaming files. Git uses diff algorithm for defining whether file was renamed or deleted/created. If difference between 2 files is more than 50%, git decides it is deleting one file and creating another. We can rename file in one commit, update in another and then everything will be fine. But as i understand such solution isn't acceptable, isn't it? I have stopped further research because it takes a lot of time, and i wait for your opinion. Thanks!

[raineorshine]

Raine, hi! I've tested all files, but i'm not confident with bindContext (i can't figure out what this feature does, i tried invoking alt + shift + b on a Thoughh and then adding children to Thought's context or to Thought itself, but nothing changes),

Yes, bindContext is incomplete, so no problem! Sorry to not let you know about this ahead of time.

cursorForward (it did nothing for me on commits before conversion and after),

cursorForward only works on mobile. When you swipe right (cursorBack), you can then swipe left (cursorForward) to effectively undo. It can also be used to navigate deeper into the hierarchy even when cursorBack has not been activated.

deleteEmptyThoughtOrOutdent (in case when we have

• parent

  • emptyThought

    • emptyThought

setting cursor on parent/<emptyThought> and pressing Backspacedo nothing (before conversion and after).

This sounds like a separate issue then. I haven't tested nested empty thoughts, so there could indeed by issues there. Could you create an issue for this?

I think we need to add tests for cursor and newThought.../newSubthoughts shortcuts in order to be confident that everything works.

What testsdo you believe are missing? newThought already has tests, and cursor is tested within existing reducers.

After conversion code passed all tests, but some issues were found out in testing all shortcuts by hand.

As you probably are aware, you are responsible for identifying exactly which commits caused the regressions.

Also i haven't succeed in renaming files. Git uses diff algorithm for defining whether file was renamed or deleted/created. If difference between 2 files is more than 50%, git decides it is deleting one file and creating another.

Well, that explains the mystery at least! Thanks for looking into it.

But as i understand such solution isn't acceptable, isn't it?

I think renaming within a separate commit would be perfectly fine. And it would have the advantage of preserving history continuity. What do you think?

[pushkov-fedor]

What testsdo you believe are missing? newThought already has tests, and cursor is tested within existing reducers.

I think they also should be tested within shortcuts. At least cursorUp and cursorDown. They have its logic before dispatching cursor action. E.g. part of cursorUp and cursorDown behavior was unexpected, because after conversion these lines of code were calculated incorrect

em/src/shortcuts/cursorUp.js

Line 32 in 07b1155

const isNotOnTheFirstLine = rangeY && parseInt(rangeY - baseNodeY - paddingTop) !== 0

em/src/shortcuts/cursorDown.js

Line 42 in 07b1155

const isNotOnTheLastLine = rangeY + rangeHeight < baseNodeY + baseNodeHeight - paddingTop - paddingBottom

[pushkov-fedor]

I think renaming within a separate commit would be perfectly fine. And it would have the advantage of preserving history continuity. What do you think?

Agree with you

[pushkov-fedor]

This sounds like a separate issue then. I haven't tested nested empty thoughts, so there could indeed by issues there. Could you create an issue for this?

Yes, will do it right now

[pushkov-fedor]

cursorForward only works on mobile. When you swipe right (cursorBack), you can then swipe left (cursorForward) to effectively undo. It can also be used to navigate deeper into the hierarchy even when cursorBack has not been activated.

Got it, thank you

[pushkov-fedor]

It's strange, but we are still having this problem. e.g. cursorBack in "Files Changed" tab is displayed as deleted and added, but actually in commits it was renamed and modified: 692f197 8c95809
Maybe it's a GitHub problem, what do you think about trying to re-open PR?

I found this PR (refined-github/refined-github#1750), there is the same problem and they solve it this way: refined-github/refined-github#1750 (comment)

[raineorshine]

That's informative, thanks. Apparently git really does just do an add+remove, and renaming is inferred using a heuristic. Maybe doing the rename in a separate PR would work. We can try that next time. For now, I'm happy moving forward with this as-is. Worst case, when tracing the history of a file it will come to an obvious stop, and we can browse the repo at the previous commit to find the renamed file manually and trace it back further if needed.

[raineorshine]

@pushkov-fedor I did some more research on git mv and renames, and these provide very good explanations:

• https://git.wiki.kernel.org/index.php/GitFaq#Why_does_Git_not_.22track.22_renames.3F
• https://stackoverflow.com/questions/1094269/whats-the-purpose-of-git-mv