cairo-dock-team team mailing list archive

[Matthieu Baerts <matttbe@xxxxxxxxx>]

Hello and thank you for this merge request!

I'll review it by adding inline comments ;-)

Diff comments:

> === modified file 'src/cairo-dock-widget.c'
> --- src/cairo-dock-widget.c	2012-10-07 00:38:20 +0000
> +++ src/cairo-dock-widget.c	2014-10-11 22:27:37 +0000
> @@ -38,9 +38,10 @@
>  
>  void cairo_dock_widget_reload (CDWidget *pCdWidget)
>  {
> -	cd_debug ("%s (type %d)", __func__, pCdWidget->iType);
> -	if (pCdWidget && pCdWidget->reload)
> -		pCdWidget->reload (pCdWidget);
> +	if (pCdWidget)
> +		cd_debug ("%s (type %d)", __func__, pCdWidget->iType);
> +		if (pCdWidget->reload)
> +			pCdWidget->reload (pCdWidget);

Nice catch! (ok, thanks to CPPCheck :-P )
But it seems you forget to add braces ('{' and '}') after your 'if' ;-)

>  }
>  
>  
> 
> === modified file 'src/gldit/cairo-dock-data-renderer.c'
> --- src/gldit/cairo-dock-data-renderer.c	2014-05-02 14:40:05 +0000
> +++ src/gldit/cairo-dock-data-renderer.c	2014-10-11 22:27:37 +0000
> @@ -192,7 +192,6 @@
>  	if (pRenderer->bWriteValues && pRenderer->bCanRenderValueAsText)
>  	{
>  		CairoDataRendererTextParam *pText;
> -		int w, h, dw, dh;

You're right, it's maybe better to reduce the scope of these variable because they are unused outer the 'if' but I think it should change nothing because GCC should optimise that correctly.
But yes, if we don't declare these variables at the beginning of the function, we should declare them at the right place :-)

>  		pText = &pRenderer->pValuesText[iNumValue];
>  		if (pText->fWidth != 0 && pText->fHeight != 0)
>  		{
> @@ -202,10 +201,10 @@
>  			glColor3f (pText->pColor[0], pText->pColor[1], pText->pColor[2]);
>  			glPushMatrix ();
>  			
> -			w = pText->fWidth * pRenderer->iWidth;
> -			h = pText->fHeight * pRenderer->iHeight;
> -			dw = w & 1;
> -			dh = h & 1;
> +			int w = pText->fWidth * pRenderer->iWidth;
> +			int h = pText->fHeight * pRenderer->iHeight;
> +			int dw = w & 1;
> +			int dh = h & 1;
>  			cairo_dock_draw_gl_text_at_position_in_area ((guchar *) pRenderer->cFormatBuffer,
>  				pFont,
>  				floor (pText->fX * iWidth) + .5*dw,
> @@ -629,9 +628,9 @@
>  	g_return_val_if_fail (pRenderer != NULL, FALSE);
>  	
>  	GldiContainer *pContainer = pIcon->pContainer;
> -	cd_debug ("Render delayed: (%s, %dx%d)", pIcon->cName, pContainer->iWidth, pContainer->iHeight);
>  	if (pContainer)
>  	{
> +		cd_debug ("Render delayed: (%s, %dx%d)", pIcon->cName, pContainer->iWidth, pContainer->iHeight);
>  		if (pContainer->iWidth == 1 && pContainer->iHeight == 1)  // container not yet resized, retry later
>  			return TRUE;
>  	
> 
> === modified file 'src/gldit/cairo-dock-dock-facility.c'
> --- src/gldit/cairo-dock-dock-facility.c	2014-08-17 22:38:25 +0000
> +++ src/gldit/cairo-dock-dock-facility.c	2014-10-11 22:27:37 +0000
> @@ -410,16 +410,9 @@
>  		iWindowPositionY = H - iNewHeight + pDock->iMaxIconHeight;
>  	
>  	int iScreenOffsetX = gldi_dock_get_screen_offset_x (pDock), iScreenOffsetY = gldi_dock_get_screen_offset_y (pDock);
> -	if (pDock->container.bIsHorizontal)
> -	{
> -		*iNewPositionX = iWindowPositionX + iScreenOffsetX;
> -		*iNewPositionY = iWindowPositionY + iScreenOffsetY;
> -	}
> -	else
> -	{
> -		*iNewPositionX = iWindowPositionX + iScreenOffsetX;
> -		*iNewPositionY = iWindowPositionY + iScreenOffsetY;
> -	}
> +	
> +	*iNewPositionX = iWindowPositionX + iScreenOffsetX;
> +	*iNewPositionY = iWindowPositionY + iScreenOffsetY;

Nice catch :-)
But iNewPositionX and iNewPositionY should be different if the dock is horizontal or vertical. I guess if the dock is vertical, we should add iScreenOffsetX to iWindowPositionY and the opposite (but we should check that carefully)

>  	//g_print ("POSITION : %d+%d ; %d+%d\n", iWindowPositionX, pDock->iScreenOffsetX, iWindowPositionY, pDock->iScreenOffsetY);
>  }
>  
> @@ -838,27 +831,9 @@
>  	gboolean bMouseInsideDock = (x_abs >= 0 && x_abs <= pDock->fFlatDockWidth && iMouseX > 0 && iMouseX < iWidth);
>  	//g_print ("bMouseInsideDock : %d (%d;%d/%.2f)\n", bMouseInsideDock, pDock->container.bInside, x_abs, pDock->fFlatDockWidth);
>  
> -	if (! bMouseInsideDock)  // hors du dock par les cotes.
> +	if (! bMouseInsideDock && iMouseY >= 0 && iMouseY < iHeight)  // hors du dock par les cotes.
>  	{
> -		if (/*cairo_dock_is_extended_dock (pDock) && */pDock->bAutoHide)  // c'est penible de sortir du dock trop facilement avec l'auto-hide.
> -		{
> -			if (iMouseY >= 0 && iMouseY < iHeight)
> -				iMousePositionType = CAIRO_DOCK_MOUSE_ON_THE_EDGE;
> -			else
> -				iMousePositionType = CAIRO_DOCK_MOUSE_OUTSIDE;
> -		}
> -		else
> -		{
> -			/**double fSideMargin = fabs (pDock->fAlign - .5) * (iWidth - pDock->fFlatDockWidth);
> -			if (x_abs < - fSideMargin || x_abs > pDock->fFlatDockWidth + fSideMargin)
> -				iMousePositionType = CAIRO_DOCK_MOUSE_OUTSIDE;
> -			else
> -				iMousePositionType = CAIRO_DOCK_MOUSE_ON_THE_EDGE;*/
> -			if (iMouseY >= 0 && iMouseY < iHeight)
> -				iMousePositionType = CAIRO_DOCK_MOUSE_ON_THE_EDGE;
> -			else
> -				iMousePositionType = CAIRO_DOCK_MOUSE_OUTSIDE;
> -		}
> +		iMousePositionType = CAIRO_DOCK_MOUSE_ON_THE_EDGE;

This one is not correct: when the mouse position is "outside" now? it should be outside if (! bMouseInsideDock && ! (iMouseY >= 0 && iMouseY < iHeight))
But I agree that if we should remove: if (pDock->bAutoHide)

>  	}
>  	else if (iMouseY >= 0 && iMouseY < iHeight)  // et en plus on est dedans en y.  //  && pPointedIcon != NULL
>  	{
> 
> === modified file 'src/gldit/cairo-dock-dock-factory.c'
> --- src/gldit/cairo-dock-dock-factory.c	2014-08-17 22:36:43 +0000
> +++ src/gldit/cairo-dock-dock-factory.c	2014-10-11 22:27:37 +0000
> @@ -1417,11 +1417,11 @@
>  		Y = x;
>  		X = y - pDock->container.iWidth/2;
>  	}
> -	int w, h;
> +	
>  	if (pDock->iInputState == CAIRO_DOCK_INPUT_AT_REST)
>  	{
> -		w = pDock->iMinDockWidth;
> -		h = pDock->iMinDockHeight;
> +		int w = pDock->iMinDockWidth;
> +		int h = pDock->iMinDockHeight;
>  		
>  		if (X <= -w/2 || X >= w/2)
>  			return FALSE;  // on n'accepte pas le drop.
> 
> === modified file 'src/gldit/cairo-dock-gui-factory.c'
> --- src/gldit/cairo-dock-gui-factory.c	2014-08-17 22:46:04 +0000
> +++ src/gldit/cairo-dock-gui-factory.c	2014-10-11 22:27:37 +0000
> @@ -841,7 +841,6 @@
>  	cd_debug ("%s (%s, %s, %s)", __func__, cGroupName, cKeyName, cOriginalConfFilePath);
>  	
>  	GSList *pList;
> -	gsize i = 0;
>  	GtkWidget *pOneWidget = pSubWidgetList->data;
>  	GError *erreur = NULL;
>  	gsize length = 0;
> @@ -858,6 +857,7 @@
>  	
>  	if (GTK_IS_SPIN_BUTTON (pOneWidget) || GTK_IS_SCALE (pOneWidget))
>  	{
> +		gsize i = 0;
>  		gboolean bIsSpin = GTK_IS_SPIN_BUTTON (pOneWidget);
>  		double *fValuesList = g_key_file_get_double_list (pKeyFile, cGroupName, cKeyName, &length, &erreur);
>  		
> @@ -1724,9 +1724,9 @@
>  	// ModuleImage
>  	int iPreviewWidth, iPreviewHeight;
>  	GdkPixbuf *pPreviewPixbuf = NULL;
> -	int w=200, h=200;
>  	if (gdk_pixbuf_get_file_info (pModule->pVisitCard->cPreviewFilePath, &iPreviewWidth, &iPreviewHeight) != NULL)  // The return value is owned by GdkPixbuf and should not be freed.
>  	{
> +		int w=200, h=200;
>  		if (iPreviewWidth > w)
>  		{
>  			iPreviewHeight *= 1.*w/iPreviewWidth;
> 
> === modified file 'src/gldit/cairo-dock-icon-manager.c'
> --- src/gldit/cairo-dock-icon-manager.c	2014-10-11 18:51:13 +0000
> +++ src/gldit/cairo-dock-icon-manager.c	2014-10-11 22:27:37 +0000
> @@ -243,7 +243,6 @@
>  	const gchar *cSuffixTab[4] = {".svg", ".png", ".xpm", NULL};
>  	gboolean bHasSuffix=FALSE, bFileFound=FALSE, bHasVersion=FALSE;
>  	GtkIconInfo* pIconInfo = NULL;
> -	int j;
>  	gchar *str = strrchr (cFileName, '.');
>  	bHasSuffix = (str != NULL && g_ascii_isalpha (*(str+1)));  // exemple : "firefox.svg", but not "firefox-3.0"
>  	bHasVersion = (str != NULL && g_ascii_isdigit (*(str+1)) && g_ascii_isdigit (*(str-1)) && str-1 != cFileName);  // doit finir par x.y, x et y ayant autant de chiffres que l'on veut.
> @@ -253,7 +252,7 @@
>  	{
>  		if (! bHasSuffix)  // test all the suffix one by one.
>  		{
> -			j = 0;
> +			int j = 0;
>  			while (cSuffixTab[j] != NULL)
>  			{
>  				g_string_printf (sIconPath, "%s/%s%s", g_cCurrentIconsPath, cFileName, cSuffixTab[j]);
> 
> === modified file 'src/gldit/cairo-dock-image-buffer.c'
> --- src/gldit/cairo-dock-image-buffer.c	2014-06-12 21:49:01 +0000
> +++ src/gldit/cairo-dock-image-buffer.c	2014-10-11 22:27:37 +0000
> @@ -517,13 +517,13 @@
>  	{
>  		// on attache la texture au FBO.
>  		///if (pContainer->iWidth == 1 && pContainer->iHeight == 1)  // container not yet fully resized
> -		if (pContainer->iWidth < pImage->iWidth || pContainer->iHeight < pImage->iHeight)
> -		{
> -			return FALSE;
> -		}
> -		iWidth = pImage->iWidth, iHeight = pImage->iHeight;
>  		if (pContainer == NULL)
>  			pContainer = g_pPrimaryContainer;
> +		if (pContainer->iWidth < pImage->iWidth || pContainer->iHeight < pImage->iHeight)
> +		{
> +			return FALSE;
> +		}
> +		iWidth = pImage->iWidth, iHeight = pImage->iHeight;
>  		if (! gldi_gl_container_make_current (pContainer))
>  		{
>  			cd_warning ("couldn't set the opengl context");
> 
> === modified file 'src/gldit/cairo-dock-themes-manager.c'
> --- src/gldit/cairo-dock-themes-manager.c	2014-10-11 22:09:58 +0000
> +++ src/gldit/cairo-dock-themes-manager.c	2014-10-11 22:27:37 +0000
> @@ -530,7 +530,10 @@
>  	//\___________________ We load launcher if needed after having removed old ones.
>  	if (! g_file_test (g_cCurrentLaunchersPath, G_FILE_TEST_EXISTS))
>  	{
> +<<<<<<< TREE
>  		g_string_printf (sCommand, "mkdir -p \"%s\"", g_cCurrentLaunchersPath);
> +=======
> +>>>>>>> MERGE-SOURCE

Better without that ;-)

>  		_launch_cmd (sCommand->str);
>  	}
>  	if (g_pMainDock == NULL || bLoadLaunchers)
> 


-- 
https://code.launchpad.net/~helene-verhaeghe27/cairo-dock-core/bugfix/+merge/238063
Your team Cairo-Dock Devs is requested to review the proposed merge of lp:~helene-verhaeghe27/cairo-dock-core/bugfix into lp:cairo-dock-core.