search

igrigorik / vimgolf

Real Vim ninjas count every keystroke - do you?
http://www.vimgolf.com/
MIT License
672 stars 65 forks source link

Replace <C-C><C-C> with <C-C> #367

Closed dstein64 closed 1 year ago

dstein64 commented 1 year ago

Vim intentionally replaces <C-C> with <C-C><C-C>, which is reflected in the output when using -W (https://github.com/vim/vim/issues/11541).

In VimGolf, when a user enters <C-C>, the input will count as two keystrokes towards the score, showing <C-C> twice in the displayed keystrokes.

This PR updates the code to replace <C-C><C-C> with <C-C>. My earlier PR handled this in keylog.rb, but I thought it would be undesirable to have this handled that way since it could result in users with old clients seeing keystrokes and scores in their client that differ from what's reported by the webapp once they upload. The approach in this PR handles the issue as soon as possible, rather than have it persist and be corrected only when keystrokes and scores are displayed.

The replacement of <C-C><C-C> to <C-C> is written to disk, since the log file is used later (when uploading). The code could be refactored to 1) avoid reading from disk twice and 2) only writing to disk when the log file has changed, but I preferred the currently implemented approach.

If you're interested in the general type of update in this PR (replacing <C-C><C-C> with <C-C>, with various possibilities for the actual implementation), please let me know if you have any requested changes.

This fixes #224.

codecov-commenter commented 1 year ago

Codecov Report

Merging #367 (011b23a) into master (77c370d) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #367   +/-   ##
=======================================
  Coverage   92.84%   92.85%           
=======================================
  Files          26       26           
  Lines         797      798    +1     
=======================================
+ Hits          740      741    +1     
  Misses         57       57
Impacted Files Coverage Δ
lib/vimgolf/lib/vimgolf/cli.rb 85.82% <100.00%> (+0.11%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dstein64 commented 1 year ago

I saw this PR was closed. @igrigorik, if you'd welcome a fix for #224, the target of this PR, please let me know if you have any implementation suggestions and/or requirements.