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.

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

[Console]::Beep() does not activate "Bell Notifications" #102211

Open
GarThor opened this issue May 14, 2024 · 6 comments · May be fixed by #102685
Open

[Console]::Beep() does not activate "Bell Notifications" #102211

GarThor opened this issue May 14, 2024 · 6 comments · May be fixed by #102685
Labels
area-System.Console help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@GarThor
Copy link

GarThor commented May 14, 2024

Description

This bug was origionally posted at microsoft/terminal#17263, however the developers on that project directed me to this one due to this code not pushing the notification:

public static void Beep()
{
const int BeepFrequencyInHz = 800;
const int BeepDurationInMs = 200;
Interop.Kernel32.Beep(BeepFrequencyInHz, BeepDurationInMs);
}
public static void Beep(int frequency, int duration)
{
const int MinBeepFrequency = 37;
const int MaxBeepFrequency = 32767;
if (frequency < MinBeepFrequency || frequency > MaxBeepFrequency)
throw new ArgumentOutOfRangeException(nameof(frequency), frequency, SR.Format(SR.ArgumentOutOfRange_BeepFrequency, MinBeepFrequency, MaxBeepFrequency));
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(duration);
Interop.Kernel32.Beep(frequency, duration);
}

Windows Terminal version
1.19.11213.0

Windows build number
10.0.19045.0

Other Software
Powershell v5 and v7

I do have oh-my-posh installed, but I don't think that affects this behavior

Flashing/notifications do appear to be configurable when using the Write-Host -NoNewLine "a"` command however.

Reproduction Steps

Steps to reproduce
o Enable "Bell notificaiton styles" in "Advanced" for your powershell profile.
o Enter the command [Console]::Beep() into the console. Observe, while the terminal does emit an audible beep, it does not alert the user in any other way (via flashing the window or the taskbar as configured).

Expected behavior

Expected Behavior
[Console]::Beep() should respect the user's configuration for bell notifications.

Actual behavior

Actual Behavior
[Console]::Beep() does not respect the user's configuration for bell notifications.

Regression?

No response

Known Workarounds

I can still use write-host "`a", which will produce the desired results.

Configuration

No response

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 14, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-console
See info in area-owners.md if you want to be subscribed.

@DHowett
Copy link

DHowett commented May 14, 2024

FWIW dotnet folks - Console.Beep uses the system Beep API on Windows, which always emits a sound. Printing U+0007 BEL is the most correct way, even on Windows (all the way down to Windows 7, if not prior), to emit a system bell since it puts the user in control of how and where the bell is received.

@adamsitnik
Copy link
Member

@DHowett thank you for your input! Do you know if switching from the Beep sys-call to writing BEL can have any other side effects?

@adamsitnik adamsitnik added help wanted [up-for-grabs] Good issue for external contributors and removed untriaged New issue has not been triaged by the area owner labels May 22, 2024
@adamsitnik adamsitnik added this to the Future milestone May 22, 2024
@alexrp
Copy link
Contributor

alexrp commented May 22, 2024

Well, for one thing, it'll make the API actually work as expected over SSH and the like.

@adamsitnik
Copy link
Member

I've marked the issue as help wanted. The person who is willing to work on the fix should do the following:

  1. Let others know that they are going to work on it.
  2. Check the current behavior of Console.Beep with redirected output (to a file for example)
  3. Install everything required to build dotnet/runtime as described in https://github.com/dotnet/runtime/blob/039d2ecb46687e89337d6d629c295687cfe226be/docs/workflow/requirements/windows-requirements.md
  4. Fork the repo, create a new branch and build dotnet/runtime
.\build.cmd -c Release -subset clr+libs+libs.tests
  1. Apply suggested fix to the overload that takes no parameters and rebuild only the System.Console.sln solution
.\dotnet.cmd build .\src\libraries\System.Console\System.Console.sln -c Release
  1. Use corerun to run a stand-alone console app to test the change
.\artifacts\bin\testhost\net9.0-windows-Release-x64\shared\Microsoft.NETCore.App\9.0.0\corerun.exe $pathToDll
  1. Ensure that nothing gets printed when the output is redirected

karakasa added a commit to karakasa/runtime that referenced this issue May 25, 2024
implement Console.Beep via BEL char,
rather than the original pinvoke.
@karakasa
Copy link
Contributor

karakasa commented May 25, 2024

I've marked the issue as help wanted. The person who is willing to work on the fix should do the following:

I would say this is a behavioral breaking change and involves some doc changes. Do you think it is necessary to fix the issue?

https://github.com/karakasa/runtime/blob/53d94f4a6c6ca1c61c968e5640e9aef0561967cd/src/libraries/System.Console/src/System/ConsolePal.Windows.cs#L678-L702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Console help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants