Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11512 closed defect (fixed)

wx accelerator separator is currently "-", not "+"

Reported by: troelsk Owned by:
Priority: normal Milestone: 2.8.11
Component: GUI-generic Version: 2.8.x
Keywords: wxCommandProcessor wxAcceleratorEntry Cc:
Blocked By: Blocking:
Patch: yes

Description

Patch:
Replace "Ctrl+" with "Ctrl-"

Topic for another patch:
Make the default wx accelerator separator "+"; it is the default everywhere as far as I know

Attachments (2)

cmdproc28.patch download (665 bytes) - added by troelsk 4 years ago.
2.8 branch
cmdproc.patch download (644 bytes) - added by troelsk 4 years ago.
Trunk

Download all attachments as: .zip

Change History (10)

Changed 4 years ago by troelsk

2.8 branch

Changed 4 years ago by troelsk

Trunk

comment:1 Changed 4 years ago by troelsk

  • Summary changed from wx accelerator separator is currently "+", not "-" to wx accelerator separator is currently "-", not "+"

comment:2 Changed 4 years ago by vadz

  • Status changed from new to confirmed

Thanks, will apply soon.

I guess we should indeed use '+' instead of '-' as I can't find any applications using the latter any more, I didn't even notice when this changed...

comment:3 Changed 4 years ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [62791]) Use wxAcceleratorEntry::ToString() for undo/redo accelerator labels.

Don't hardcode the string representation of the accelerators, it was
inconsistent with the rest of wxWidgets which used '-' while here a '+' was
used.

Closes #11512.

comment:4 follow-up: Changed 4 years ago by troelsk

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • r62791: I think \t must stay.
  • Ticket has patch for stable 2.8 branch also, safe to apply as it is tested and all

comment:5 in reply to: ↑ 4 Changed 4 years ago by vadz

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to troelsk:

  • r62791: I think \t must stay.

Oops, sorry, thanks for noticing this!

  • Ticket has patch for stable 2.8 branch also, safe to apply as it is tested and all

Sorry, I don't want to change this in 2.8. It's hardly a fatal bug and you never know what can be broken by this (maybe someone relies on it being '+' in their code, I've seen weirder things happen).

comment:6 Changed 4 years ago by VZ

(In [62794]) Restore TAB in accelerator string wrong removed by r62791.

See #11512.

comment:7 Changed 4 years ago by troelsk

Sorry, I don't want to change this in 2.8. It's hardly a fatal bug...

Okay, I'll patch my local wx copy then, already a hackorama extraordinaire, sigh :-(
Or deal with it in user space. Naturally I would have preferred both patches to be applied, to both branches, with no tinkering...

comment:8 Changed 4 years ago by troelsk

'+' is now the wx default, in trunk, since r64157

Note: See TracTickets for help on using tickets.