-
Notifications
You must be signed in to change notification settings - Fork 184
Add system property to disable beep sounds #2614
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.
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot the evaluation of
!"off".equalsIgnoreCase(System.getProperty(BEEP_ENABLED))
should be stored in a public static boolean field, we do not need to change it dynamically.
We can also remove the testcase, this can not be tested automatically.
Done in commit e9c2a1d. Changed BEEP_ENABLED to a public static boolean field initialized with the property evaluation. Also removed the test cases as requested. |
Co-authored-by: laeubi <[email protected]>
…t cases Co-authored-by: laeubi <[email protected]>
41ab962
to
f1622e5
Compare
Test Results104 files - 11 104 suites - 11 8s ⏱️ - 11m 3s Results for commit f1622e5. ± Comparison against base commit 6fa4ee7. This pull request removes 4466 tests.
|
Why not use off as default? |
Just because we (personally) decide we don’t like beep enough to add a system property to disable it completely should we really decide that to be the new default for everyone? It’s arguably not even for everyone’s safety and comfort. It’s more of a preference. I generally turn my speakers off when not using the them. So I don’t personally care, but I also see no reason force a new default on everyone else. |
Please see general discussion here: |
Yes I saw that. I don’t agree. A system property does no harm. Defaults are a different story, hence my comment here. Deprecation would be a different PR. This seems to have shifted from one topic to another where there appears now to be a compelling reason for enforcing correctness on everyone. That seems questionable to me, and I hate my computer making noises. |
@merks I would be fine with either way, I'm just sick of people complaining my application has "bugs" while it is a "feature" of platform ;-) So @merks @vogella it would be good to leaver you concerns or support at the issue so we hopefully can came to a conclusion there and I'll create a PR with whatever decision is taken there. |
I'm in favor of a "non-annoying" default—therefore, no beeping. I have yet to meet anyone who actually likes applications that make beeping sounds. When I use Eclipse with sound enabled, people regularly ask me to mute my computer. I believe we should choose reasonable defaults for Eclipse once we identify an issue, rather than fixing it in a way that only experts can handle (such as through obscure properties). That said, having such an option is still better than nothing, so I’m not blocking progress either way. |
I like this proposal. |
I don't think we should make IMO, all calls to Display.beep() in JDT need to be replaced by either no action (if beeping means "nothing to do"), or a regular logging (if we actually don't want to interrupt user) or alerts (if this is a problem worth alerting users). |
Well the system property is a kind of "preference" for the sake of SWT as a library that can run without OSGi and especially without Eclipse. Nerveless of course we can also have an API to toggle the switch so it can than be used in a preference dialog (even though I would not bother to implement such feature)... but sad enough now discussion has moved to this PR (what is only *one possible way to implement it) instead of taking place in the initial issue, but anyways it does not matter to me if we come to a conclusion somehow. So to summarize we have:
So my suggestion would be now:
|
I would favor "We decide on some places where beep is used in application code if it is really usefull to call the method at all." (and basically for JDT we get rid of about all calls to |
@mickaelistria this still does not solve it for other plugins and I want something that only depends on SWT... if JDT or others ever move ... who knows? Can I convince every SWT lirary to adapt and offer me preferences? Likley not... Do I want to maintan a lot of preferences in my application instead of setting one system property? Likey also not managable. |
Overview
This PR implements a new system property
swt.beep
to control beep sound behavior in SWT Display across all platforms (macOS/Cocoa, Windows/Win32, and Linux/GTK).Motivation
Currently, the Display class provides a
beep()
method that emits a system sound. In some cases, applications may want to disable these beep sounds regardless of what library code calls this method. Previously, the only workarounds were to mute the system speaker or configure OS sound settings, which are not acceptable solutions for many use cases.Implementation
The implementation adds a public static boolean field that is evaluated once at class initialization:
swt.beep
public static final boolean BEEP_ENABLED
"off"
(case-insensitive) - disables beep sounds completely"on"
or any other value - enables beep sounds (default behavior)The property is evaluated once when the Display class is loaded, not dynamically on each beep call.
Changes per platform:
macOS (Cocoa): Added
BEEP_ENABLED
field and modifiedbeep()
to check it before callingOS.NSBeep()
Windows (Win32): Added
BEEP_ENABLED
field and modifiedbeep()
to check it before callingOS.MessageBeep(OS.MB_OK)
Linux (GTK): Added
BEEP_ENABLED
field and modifiedbeep()
to check it before callingGDK.gdk_display_beep()
Usage
Note: The system property must be set before the Display class is loaded, as the value is cached at initialization.
Benefits
BEEP_ENABLED
field is public, allowing external code to check if beeps are enabledImplementation Notes
beep()
method implementation and added a public static boolean field per platformFixes #
Original prompt
Fixes #2613
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.