Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature: extend functional keys handling with key modifiers #705

Merged
merged 6 commits into from Dec 12, 2023
Merged

Feature: extend functional keys handling with key modifiers #705

merged 6 commits into from Dec 12, 2023

Conversation

penguinolog
Copy link
Collaborator

@penguinolog penguinolog commented Dec 12, 2023 •
edited

  • F1 is a special and use number 1 postfixed by P
  • F2 is a special and use number 1 postfixed by Q
  • F3 is a special and use number 1 postfixed by R
  • F4 is a special and use number 1 postfixed by S

Related #702
Related #702

Checklist
  • I've ensured that similar functionality has not already been implemented
  • I've ensured that similar functionality has not earlier been proposed and declined
  • I've branched off the master or python-dual-support branch
  • I've merged fresh upstream into my branch recently
  • I've ran tox successfully in local environment
  • I've included docstrings and/or documentation and/or examples for my code (if this is a new feature)

@penguinolog penguinolog added the Feature Feature request/implementation label Dec 12, 2023
@penguinolog
Copy link
Collaborator Author

penguinolog commented Dec 12, 2023 •
edited

Before:
image

After:
image

(Locally I modified example to provide hex key output instead of decimal)

@coveralls
Copy link

coveralls commented Dec 12, 2023 •
edited

Pull Request Test Coverage Report for Build 7185062239

  • 32 of 42 (76.19%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 70.541%
Changes Missing Coverage Covered Lines Changed/Added Lines %
urwid/escape.py 32 42 76.19%
Files with Coverage Reduction New Missed Lines %
urwid/escape.py 1 67.22%
Totals Coverage Status
Change from base Build 7176603581: 0.2%
Covered Lines: 7603
Relevant Lines: 10843
💛 - Coveralls

@penguinolog penguinolog force-pushed the part_f_keys branch 3 times, most recently from 8d7b3d8 to 5a9699d Compare December 12, 2023 12:38
penguinolog
penguinolog commented Dec 12, 2023
View reviewed changes
urwid/escape.py
@@ -476,14 +494,14 @@ def process_keyqueue(codes: Sequence[int], more_available: bool) -> tuple[list[s
else:
return [f"<{code:d}>"], codes[1:]

for i in range(need_more):
if len(codes) - 1 <= i:
for i in range(1, need_more + 1):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

simplify indexing

penguinolog
penguinolog commented Dec 12, 2023
View reviewed changes
urwid/escape.py
@@ -459,13 +468,22 @@ def process_keyqueue(codes: Sequence[int], more_available: bool) -> tuple[list[s

em = str_util.get_byte_encoding()

if em == "wide" and code < 256 and within_double_byte(code.to_bytes(1, "little"), 0, 0):
if (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

force fit in 12 symbols

penguinolog
penguinolog commented Dec 12, 2023
View reviewed changes
urwid/escape.py Outdated
for digit in "12345678"
for num in (1, range(5, 12))
),
*((f"[1;{digit}Q", escape_modifier(digit) + "f2") for digit in "12345678"), # F2 is special
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

incorrect parse of this sequence always cause exit from "input_test"

@penguinolog
Feature: extend functional keys handling with key modifiers
97ded00 
* F1 is a special and use number `1` postfixed by P
* F2 is a special and use number `1` postfixed by Q
* F3 is a special and use number `1` postfixed by R
* F4 is a special and use number `1` postfixed by S

Related urwid#702
penguinolog
penguinolog commented Dec 12, 2023
View reviewed changes
urwid/escape.py
action = "drag"
else:
action = "press"
else:
elif action == "m":
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://invisible-island.net/xterm/ctlseqs/ctlseqs.pdf page 46

penguinolog
penguinolog commented Dec 12, 2023
View reviewed changes
urwid/escape.py
@@ -289,9 +312,10 @@ def read_sgrmouse_info(self, keys, more_available: bool):
raise MoreInputRequired()
return None

(b, x, y) = value[:-1].split(";")
(b, x, y) = (int(val) for val in value[:-1].split(";"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let's cast to int in-place

penguinolog
penguinolog commented Dec 12, 2023
View reviewed changes
urwid/escape.py
(f"O{digit}{letter}", escape_modifier(digit) + key)
# modified F1-F4 keys - O#X form and [1;#X form
(prefix + digit + letter, escape_modifier(digit) + f"f{number}")
for prefix in ("O", "[1;")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

figured out, that generator for another prefix was available

wardi
wardi reviewed Dec 12, 2023
View reviewed changes
urwid/escape.py Outdated Show resolved Hide resolved
@penguinolog @wardi
Update urwid/escape.py
b8f1752 
Co-authored-by: Ian Ward <ian@excess.org>
@wardi
Copy link
Collaborator

wardi commented Dec 12, 2023

think it's worth adding tests of the parsing code here to make sure it doesn't break?

@penguinolog
Copy link
Collaborator Author

Probably yes. will add in 1 hour and fix "ruff" checker

@penguinolog
Add tests for modifiers
049832a 
* fix key `F20` in generator
@github-actions github-actions bot added the Tests Tests label Dec 12, 2023
@penguinolog penguinolog merged commit 47ff954 into urwid:master Dec 12, 2023
35 checks passed
@penguinolog penguinolog deleted the part_f_keys branch December 12, 2023 17:33
@penguinolog penguinolog mentioned this pull request Dec 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wardi wardi wardi approved these changes

@ulidtko ulidtko Awaiting requested review from ulidtko

@tonycpsu tonycpsu Awaiting requested review from tonycpsu

Assignees
No one assigned
Labels
Feature Feature request/implementation Tests Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@penguinolog @coveralls @wardi