Re: [PUSHED][PATCH (partial)] fdo#48549 System::Beep() removal

[Noel Power]

pushed, thanks !!

Noel
On 28/07/12 23:06, Mathieu Vonlanthen wrote:

Hi,

I've written a new patch cleaning remaining calls to Sound::Beep and
unused include. I've also removed sound.hxx and sound.cxx as they were
no more used.

I've built master with this patch without warnings on the modified files
but there is still no logging added.

There is still calls to GetDisplay()->Beep() and gdk_display_beep(
getGdkDisplay() ), they are not directly related to fdo#48549 but they
maybe deserve to be removed too?

Best Regards

Mathieu



___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Mathieu Vonlanthen]

Hi,

I've written a new patch cleaning remaining calls to Sound::Beep and
unused include. I've also removed sound.hxx and sound.cxx as they were
no more used.

I've built master with this patch without warnings on the modified files
but there is still no logging added.

There is still calls to GetDisplay()->Beep() and gdk_display_beep(
getGdkDisplay() ), they are not directly related to fdo#48549 but they
maybe deserve to be removed too?

Best Regards

Mathieu

-- 
http://www.fastmail.fm - Accessible with your email software
  or over the web

From 8f43b49730c2d496041029062f7951a427b21b17 Mon Sep 17 00:00:00 2001
From: Mathieu Vonlanthen 
Date: Sat, 28 Jul 2012 16:13:18 +0200
Subject: [PATCH] Bug 48549 - System::Beep() removal

Change-Id: I011048912af051a762a78af8646513a1fc624073
---
 basctl/source/basicide/baside2.cxx |1 -
 basctl/source/basicide/baside2b.cxx|1 -
 basctl/source/basicide/basides2.cxx|1 -
 basctl/source/basicide/brkdlg.cxx  |1 -
 basctl/source/basicide/objdlg.cxx  |1 -
 basic/source/runtime/methods.cxx   |1 -
 basic/source/sbx/sbxobj.cxx|1 -
 cui/source/dialogs/colorpicker.cxx |1 -
 cui/source/dialogs/cuicharmap.cxx  |5 +--
 cui/source/dialogs/cuitbxform.cxx  |1 -
 formula/source/ui/dlg/funcutl.cxx  |1 -
 sc/source/core/data/documen4.cxx   |1 -
 sc/source/ui/app/inputhdl.cxx  |1 -
 sc/source/ui/docshell/docfunc.cxx  |1 -
 sc/source/ui/docshell/olinefun.cxx |1 -
 sc/source/ui/navipi/content.cxx|1 -
 sc/source/ui/navipi/navipi.cxx |1 -
 sc/source/ui/view/dbfunc3.cxx  |1 -
 sc/source/ui/view/editsh.cxx   |1 -
 sc/source/ui/view/gridwin.cxx  |1 -
 sc/source/ui/view/gridwin2.cxx |1 -
 sc/source/ui/view/preview.cxx  |1 -
 sc/source/ui/view/select.cxx   |1 -
 sc/source/ui/view/tabcont.cxx  |1 -
 sc/source/ui/view/tabvwshe.cxx |1 -
 sc/source/ui/view/viewfun2.cxx |1 -
 sc/source/ui/view/viewfun6.cxx |1 -
 sc/source/ui/view/viewfunc.cxx |1 -
 .../controller/SlsSelectionFunction.cxx|1 -
 sfx2/source/appl/app.cxx   |1 -
 sfx2/source/appl/appcfg.cxx|1 -
 sfx2/source/dialog/passwd.cxx  |2 -
 starmath/source/dialog.cxx |1 -
 svtools/source/contnr/fileview.cxx |1 -
 svtools/source/contnr/svlbitm.cxx  |1 -
 svtools/source/edit/textview.cxx   |1 -
 svx/inc/svx/galctrl.hxx|1 -
 svx/source/fmcomp/gridctrl.cxx |1 -
 svx/source/form/fmexpl.cxx |1 -
 svx/source/form/fmshell.cxx|1 -
 svx/source/form/tbxform.cxx|1 -
 svx/source/gengal/gengal.cxx   |1 -
 svx/workben/edittest.cxx   |1 -
 sw/source/core/doc/docedt.cxx  |3 --
 sw/source/core/layout/paintfrm.cxx |1 -
 sw/source/core/text/frmpaint.cxx   |1 -
 sw/source/ui/dochdl/swdtflvr.cxx   |1 -
 sw/source/ui/utlui/content.cxx |1 -
 sw/source/ui/wrtsh/wrtsh1.cxx  |1 -
 vcl/Library_vcl.mk |1 -
 vcl/Package_inc.mk |1 -
 vcl/inc/vcl/msgbox.hxx |1 -
 vcl/inc/vcl/sound.hxx  |   41 
 vcl/source/app/dbggui.cxx  |2 -
 vcl/source/app/sound.cxx   |   40 ---
 vcl/source/control/field2.cxx  |1 -
 vcl/source/control/scrbar.cxx  |7 
 vcl/source/control/tabctrl.cxx |1 -
 vcl/source/window/menu.cxx |1 -
 vcl/source/window/msgbox.cxx   |8 
 vcl/source/window/syswin.cxx   |4 --
 vcl/source/window/toolbox.cxx  |2 -
 vcl/source/window/winproc.cxx  |8 +---
 63 files changed, 2 insertions(+), 172 deletions(-)
 delete mode 100644 vcl/inc/vcl/sound.hxx
 delete mode 100644 vcl/source/app/sound.cxx

diff --git a/basctl/source/basicide/baside2.cxx b/basctl/source/basicide/baside2.cxx
index 5f7fe56a..1eab598 100644
--- a/basctl/source/basicide/

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Stephan Bergmann]

On 07/23/2012 05:42 PM, Eike Rathke wrote:

On Monday, 2012-07-23 16:01:56 +0200, Philipp Riemer wrote:

But I do not know what the general opinion of the LO developers for
that cases is... Maybe they are happy without any logging in there...
Whould love to hear from the others what they think.


In general such cases deserve a SAL_WARN,


...or, depending on the nature of the event that caused the beep, merely 
a SAL_LOG.  See 
 
for a (potentially unhelpful) differentiation of the two's intended uses.


The general idea is that SAL_WARN is for events that "do not normally 
happen."  And something like "in the Basic IDE, user attempts to set a 
breakpoint on a line that contains no code" (whether or not that is 
something that used to trigger a beep; I did not check) in my 
interpretation does not fall into that category and thus should be 
logged with SAL_LOG.  But a somewhat gray area indeed...


Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Eike Rathke]

Hi Philipp,

On Monday, 2012-07-23 16:01:56 +0200, Philipp Riemer wrote:

> But I do not know what the general opinion of the LO developers for
> that cases is... Maybe they are happy without any logging in there...
> Whould love to hear from the others what they think.

In general such cases deserve a SAL_WARN, so that at least in a debug
build some output is logged to the console.

  Eike

-- 
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD


pgpyPcVfJc0oL.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Philipp Riemer]

Hey,

2012/7/23 Mathieu Vonlanthen :
> Hi,
> Here is my license statement:
> -
> All of my past & future contributions to LibreOffice may be licensed
> under the MPL/LGPLv3+ dual license.
> --
> I send a new version of the patch taking into account the last commits.
> I didn't add error reporting because I don't know the official way to do
> this.
>
> Best regards
>
> Mathieu Vonlanthen

Thank you very much for your work! Having reviewed your patch, I still
think that a warning/logging might be helpful for latter debugging in
the following files:

* basctl/source/basicide/baside2.cxx
  - @@ -572,9 +571,6 @@ sal_Bool ModulWindow::ToggleBreakPoint(
sal_uLong nLine )
  - @@ -790,10 +782,7 @@ void ModulWindow::BasicAddWatch()
  - @@ -1022,10 +1011,6 @@ void ModulWindow::ExecuteCommand( SfxRequest& rReq )
* basic/source/runtime/methods.cxx
* formula/source/ui/dlg/funcutl.cxx
* sc/source/ui/app/inputhdl.cxx
* sc/source/ui/docshell/olinefun.cxx
* sc/source/ui/navipi/content.cxx
* sc/source/ui/navipi/navipi.cxx
* sc/source/ui/view/viewfun2.cxx
* sc/source/ui/view/viewfun6.cxx
* svtools/source/edit/textview.cxx
* svx/source/form/tbxform.cxx
* svx/workben/edittest.cxx
* sw/source/ui/utlui/content.cxx

But I do not know what the general opinion of the LO developers for
that cases is... Maybe they are happy without any logging in there...
Whould love to hear from the others what they think.

Nevertheless, removing that annoying beeps is a good thing!

Cheers,
Philipp
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Mathieu Vonlanthen]

Hi,
Here is my license statement:
-
All of my past & future contributions to LibreOffice may be licensed
under the MPL/LGPLv3+ dual license.
--
I send a new version of the patch taking into account the last commits.
I didn't add error reporting because I don't know the official way to do
this.

Best regards

Mathieu Vonlanthen

-- 
http://www.fastmail.fm - IMAP accessible web-mail

From 499921f466ae4db5aeb3ccdc9c8c59e98b290cfe Mon Sep 17 00:00:00 2001
From: Mathieu Vonlanthen 
Date: Thu, 19 Jul 2012 11:02:45 +0200
Subject: [PATCH] fdo#48549 System::Beep() removal

Change-Id: I8fe133dd8d1f759fbe21d47ae358c0b5451812b5
---
 basctl/source/basicide/baside2.cxx  |   17 +
 basctl/source/basicide/baside2b.cxx |   22 +---
 basctl/source/basicide/brkdlg.cxx   |1 -
 basic/source/runtime/methods.cxx|1 -
 formula/source/ui/dlg/funcutl.cxx   |4 ---
 sc/source/core/data/documen4.cxx|2 --
 sc/source/ui/app/inputhdl.cxx   |6 -
 sc/source/ui/docshell/docfunc.cxx   |6 -
 sc/source/ui/docshell/olinefun.cxx  |8 --
 sc/source/ui/navipi/content.cxx |2 --
 sc/source/ui/navipi/navipi.cxx  |2 --
 sc/source/ui/view/editsh.cxx|   12 -
 sc/source/ui/view/gridwin.cxx   |   10 
 sc/source/ui/view/gridwin2.cxx  |2 --
 sc/source/ui/view/select.cxx|1 -
 sc/source/ui/view/tabcont.cxx   |3 ---
 sc/source/ui/view/tabvwshe.cxx  |1 -
 sc/source/ui/view/viewfun2.cxx  |4 ---
 sc/source/ui/view/viewfun6.cxx  |   47 ++-
 sc/source/ui/view/viewfunc.cxx  |2 --
 svtools/source/contnr/fileview.cxx  |3 ---
 svtools/source/contnr/svlbitm.cxx   |2 --
 svtools/source/contnr/svtreebx.cxx  |1 -
 svtools/source/edit/textview.cxx|5 
 svx/source/fmcomp/gridctrl.cxx  |1 -
 svx/source/form/tbxform.cxx |1 -
 svx/workben/edittest.cxx|2 --
 sw/source/ui/utlui/content.cxx  |6 +
 vcl/source/control/field2.cxx   |7 +-
 vcl/source/window/menu.cxx  |   13 --
 30 files changed, 12 insertions(+), 182 deletions(-)

diff --git a/basctl/source/basicide/baside2.cxx b/basctl/source/basicide/baside2.cxx
index d427a30..0c91e85 100644
--- a/basctl/source/basicide/baside2.cxx
+++ b/basctl/source/basicide/baside2.cxx
@@ -546,7 +546,6 @@ sal_Bool ModulWindow::ToggleBreakPoint( sal_uLong nLine )
 CheckCompileBasic();
 if ( aStatus.bError )
 {
-Sound::Beep();
 return sal_False;
 }
 
@@ -572,9 +571,6 @@ sal_Bool ModulWindow::ToggleBreakPoint( sal_uLong nLine )
 }
 }
 }
-
-if ( !bNewBreakPoint )
-Sound::Beep();
 }
 }
 
@@ -753,7 +749,6 @@ void ModulWindow::BasicAddWatch()
 {
 DBG_CHKTHIS( ModulWindow, 0 );
 String aWatchStr;
-sal_Bool bInserted = sal_False;
 AssertValidEditEngine();
 sal_Bool bAdd = sal_True;
 if ( !GetEditView()->HasSelection() )
@@ -780,9 +775,6 @@ void ModulWindow::BasicAddWatch()
 bInserted = sal_True;
 }
 }
-
-if ( !bInserted )
-Sound::Beep();
 }
 
 
@@ -790,10 +782,7 @@ void ModulWindow::BasicAddWatch()
 void ModulWindow::BasicRemoveWatch()
 {
 DBG_CHKTHIS( ModulWindow, 0 );
-bool bRemoved = pLayout->GetWatchWindow().RemoveSelectedWatch();
-
-if ( !bRemoved )
-Sound::Beep();
+pLayout->GetWatchWindow().RemoveSelectedWatch();
 }
 
 
@@ -1022,10 +1011,6 @@ void ModulWindow::ExecuteCommand( SfxRequest& rReq )
 }
 break;
 case SID_BASICIDE_MATCHGROUP:
-{
-if ( !GetEditView()->MatchGroup() )
-Sound::Beep();
-}
 break;
 case SID_BASICIDE_TOGGLEBRKPNT:
 {
diff --git a/basctl/source/basicide/baside2b.cxx b/basctl/source/basicide/baside2b.cxx
index 0e63635..d10859c 100644
--- a/basctl/source/basicide/baside2b.cxx
+++ b/basctl/source/basicide/baside2b.cxx
@@ -1496,8 +1496,6 @@ IMPL_LINK( WatchWindow, EditAccHdl, Accelerator *, pAcc )
 aXEdit.SetSelection( Selection( 0, 0x ) );
 UpdateWatches();
 }
-else
-Sound::Beep();
 }
 break;
 case KEY_ESCAPE:
@@ -1942,9 +1940,6 @@ sal_Bool WatchTreeListBox::EditingEntry( SvLBoxEntry* pEntry, Selection& )
 }
 }
 
-if ( !bEdit )
-Sound::Beep();
-
 return bEdit;
 }
 
@@ -1962,22 +1957,10 @@ sal_Bool WatchTreeListBox::EditedEntry( SvLBoxEntry* pEntry, const rtl::OUString
 aResult = aResult.Copy( 1, nResultLen - 2 );
 
 sal_Bool bResModified = ( aResult != aEditingRes ) ? sal_True : sal_False;
-sal_Bool bError = sal_False;
-if ( !aVName.Len() )
-{
-bError = sal_True;
-}
-
 sal_Bool bRet = sal_False;
 
-if ( bError )
-{
-Sound::Beep();
-  

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Mek Otar]

> It'd be great to have a blanket MPL/LGPLv3+ license statement from you

Mek Otar is my nickname, I will send you a license statement with my
real name: Mathieu Vonlanthen.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Philipp Riemer]

2012/7/20 Mek Otar :
> Hi,
>
> I've written a patch that remove all call to Sound:Beep. Thomas
> Arnhold suggest me to replace the Sound:Beep by some error reporting
> mechanism. My question is: What is, in your opinion, the best way to
> do this?
>
> Best regards
>
> Mek Otar

Hi Mek,

When I looked at your patch I was also wondering if such a radical
deletion is such a good thing... or if it wouldn't be better to log at
least in some of those cases an error messages? In my eyes these beeps
were originally intended to tell the user that something unexpected
happend but I agree that a beep is not the right way to do so.

Cheers,
Philipp
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH (partial)] fdo#48549 System::Beep() removal

[Michael Meeks]

Hi Mek,

On Fri, 2012-07-20 at 16:54 +0200, Mek Otar wrote:
> I've written a patch that remove all call to Sound:Beep. Thomas
> Arnhold suggest me to replace the Sound:Beep by some error reporting
> mechanism. My question is: What is, in your opinion, the best way to
> do this?

Looks great to me, the beeping is random, turns into XBell on Linux so
for multi-user seats it can come out of a totally different machine,
somewhere else ;-) etc.

It'd be great to have a blanket MPL/LGPLv3+ license statement from you
on the list that we can link into here:

http://wiki.documentfoundation.org/Development/Developers

Than I'd merge it ! :-)

Thanks for your contribution; what's next ? :-)

All the best,

Michael.

-- 
michael.me...@suse.com  <><, Pseudo Engineer, itinerant idiot

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[PATCH (partial)] fdo#48549 System::Beep() removal

[Mek Otar]

Hi,

I've written a patch that remove all call to Sound:Beep. Thomas
Arnhold suggest me to replace the Sound:Beep by some error reporting
mechanism. My question is: What is, in your opinion, the best way to
do this?

Best regards

Mek Otar


0001-fdo-48549-System-Beep-removal.patch
Description: Binary data
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice