naxuroqa / Venom

a modern Tox client for the GNU/Linux desktop
GNU General Public License v3.0
282 stars 65 forks source link

VENOM-389: Fix modifiers issue #390

Closed SkyzohKey closed 6 years ago

SkyzohKey commented 6 years ago

closes #389

codecov-io commented 6 years ago

Codecov Report

Merging #390 into develop will decrease coverage by 0.01%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop    #390      +/-   ##
==========================================
- Coverage    24.62%   24.6%   -0.02%     
==========================================
  Files          101     101              
  Lines         5938    5942       +4     
==========================================
  Hits          1462    1462              
- Misses        4476    4480       +4
Impacted Files Coverage Δ
src/view/ConversationWindow.vala 0.53% <0%> (-0.02%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9c69c2c...9483223. Read the comment docs.

SkyzohKey commented 6 years ago

I guess I could replace the SHIFT_MASK with key.accel_mods

naxuroqa commented 6 years ago

If you replace the shift_mask with accel_mods then we are pretty much where we were before :)

SkyzohKey commented 6 years ago

Mh

SkyzohKey commented 6 years ago

What is needed for this to be merged?

naxuroqa commented 6 years ago

There are still requested changes that need to be adressed. Please don't feature creep in bug fixes, or if you do, at least provide reasoning why you think this improves the existing behaviour.

The current implementation, though broken, catches a user definable key combination (key+modifier(s)), consumes the event on an exact match.

This means I could define something like ctrl-s as a key combination to send messages.

Your change would change the behavior, you would catch any key combination containing the user defined key and ignore the user defined modifier(s) all together. Additionally would the user defined key and shift always insert a newline.

This means that if someone sets the send key accel to ctrl-s, suddenly pressing s would send a message and shift-s would insert a newline (what?).

I hope this explains my concerns here.