search

dail8859 / SurroundSelection

Notepad++ plugin to automatically surround the selection in quotes/brackets/parenthesis
GNU General Public License v2.0
55 stars 5 forks source link

To be independent of keyboard layout; replaced VK codes with ASCII chars. #6

Closed visorz closed 5 years ago

visorz commented 5 years ago

Hello, this is my first pull request ever. I changed the Main.cpp to compare the typed ASCII symbols with given symbols. That works over different languages and keyboard layouts. All code I added uses self-explaining variable names, named state comparisons and extracted methods. Overall the code is shorter and easier to extend.

dail8859 commented 5 years ago

Hi and thanks for the PR! Does this in theory do the same thing as PR #5?

visorz commented 5 years ago

PR #5 uses the same mechanism: translating the input key to an ASCII character and then comparing. I had the idea independently of #5 and it was a conincidence, that we both worked on the same layout independence. My PR #6 includes all braces that have been added in PR #5 but I did not update the Readme file in my fork. That is why I did not include it in the commit.

My code does not include goto and does not use if .. else .. statements for all possible combinations. Therefore I am convinced, that my code is easier to read and to maintain. Please compile it and try it yourself.

visorz commented 5 years ago

There is no hint to what caused the continous-integration build to fail. Can someone please restart the build?

dail8859 commented 5 years ago

does not use if .. else .. statements for all possible combinations. Therefore I am convinced, that my code is easier to read and to maintain

Honestly I was actually a bit confused why SetPrePostCharacters() was called consecutively when a normal if ... else statement should suffice.

There is no hint to what caused the continous-integration build to fail.

Appveyor reported Cannot fail over job to clouds/groups 'pro-backup'. All clouds within the group are either failing, or offline, or busy so looks like its not related to the code at all.

Since both this and #5 do nearly the exact same thing there really isn't a way to incorporate both. I do greatly appreciate your time and PR but I'll merge in #5 in favor of this one.