if monitor name happens to get updated in ParseOptions, the rest of FvwmPager config is skipped #146

d-e-e-p · 2020-06-28T11:44:21Z

might be useful to have a helper function update monitor params from string, eg:

void extract_monitor_config(struct fpmonitor *m, char *tline) {               
    int  output, mdw, mdh, vx, vy, vxmax, vymax, iscur;                       
    int  x, y, w, h;                                                          
                                                                              
    sscanf(tline, "%d %d %d %d %d %d %d %d %d %d %d %d",                      
        &output, &iscur, &mdw, &mdh, &vx, &vy, &vxmax, &vymax,                
        &x, &y, &w, &h);                                                      
    m->x = x;                                                                 
    m->y = y;                                                                 
    m->w = w;                                                                 
    m->h = h;                                                                 
    m->output = output;                                                       
    m->is_current = iscur;                                                    
    m->virtual_scr.MyDisplayWidth = mdw;                                      
    m->virtual_scr.MyDisplayHeight = mdh;                                     
    m->virtual_scr.Vx = vx;                                                   
    m->virtual_scr.Vy = vy;                                                   
    m->virtual_scr.VxMax = vxmax;                                             
    m->virtual_scr.VyMax = vymax;                                             
                                                                              
}

…onfig file is skipped.

ThomasAdam · 2020-06-28T12:46:26Z

Hi @d-e-e-p,

If you wanted to incorporate that suggestion, please do.

This would mean using this function in two places:

list_config_info()
ParseOptions()

…s from string

ThomasAdam

Hi @d-e-e-p,

Excellent. Thanks for this. Functionally this all looks good. Just some minor style nits inline, with a couple of suggestions you can approve as you're reading.

ThomasAdam · 2020-06-29T07:15:44Z

modules/FvwmPager/FvwmPager.c

@@ -1467,6 +1467,28 @@ void list_end(void)
    XFree((char *)children);
 }

+void extract_monitor_config(struct fpmonitor *m, char *tline) {


Suggested change

void extract_monitor_config(struct fpmonitor *m, char *tline) {

void extract_monitor_config(struct fpmonitor *m, char *tline)

{

ThomasAdam · 2020-06-29T07:16:33Z

modules/FvwmPager/FvwmPager.c

-				m2->virtual_scr.Vy = vy;
-				m2->virtual_scr.VxMax = vxmax;
-				m2->virtual_scr.VyMax = vymax;
+                extract_monitor_config(m2,tline);


The indenting here looks odd.

ThomasAdam · 2020-06-29T07:16:47Z

modules/FvwmPager/FvwmPager.c

-		m->virtual_scr.Vy = vy;
-		m->virtual_scr.VxMax = vxmax;
-		m->virtual_scr.VyMax = vymax;
+        extract_monitor_config(m,tline);


Same here -- indentation looks odd. Also:

Suggested change

extract_monitor_config(m,tline);

extract_monitor_config(m, tline);

ThomasAdam · 2020-06-29T07:17:30Z

modules/FvwmPager/FvwmPager.c

 		TAILQ_INSERT_TAIL(&fp_monitor_q, m, entry);
+


No blank line necessary here:

Suggested change

ThomasAdam · 2020-06-29T07:18:03Z

modules/FvwmPager/FvwmPager.c

-			    m2->virtual_scr.Vy = vy;
-			    m2->virtual_scr.VxMax = vxmax;
-			    m2->virtual_scr.VyMax = vymax;
+                extract_monitor_config(m2,next); 


Again, indentation seems off. Also:

Suggested change

extract_monitor_config(m2,next);

extract_monitor_config(m2, next);

ThomasAdam · 2020-06-29T07:18:21Z

modules/FvwmPager/FvwmPager.c

-	    m->virtual_scr.Vy = vy;
-	    m->virtual_scr.VxMax = vxmax;
-	    m->virtual_scr.VyMax = vymax;
+        extract_monitor_config(m,next); 


Indentation.

ThomasAdam · 2020-06-29T07:19:30Z

modules/FvwmPager/FvwmPager.h

@@ -169,6 +169,7 @@ RETSIGTYPE DeadPipe(int nonsense);
 void process_message(FvwmPacket*);
 void ParseOptions(void);

+void extract_monitor_config(struct fpmonitor *m, char *tline);


I know it's a mess of styles, but for new additions:

Scoped functions should be declared as static;

Function prototypes can be unnamed with respect to the variables used.

Hence:

Suggested change

void extract_monitor_config(struct fpmonitor *m, char *tline);

static void extract_monitor_config(struct fpmonitor *, char *);

Co-authored-by: Thomas Adam <[email protected]>

d-e-e-p · 2020-06-29T21:12:11Z

check in with changes..thanks for walking me thru all of them.

ThomasAdam · 2020-06-29T21:42:41Z

Hi @d-e-e-p,

OK. Thanks! Please can you squash the last two commits together and push the changes here?

Thanks,
Thomas

…onfig file is skipped. also added helper function extract_monitor_config to extract monitor params from string

if monitor name happens to get updated in ParseOptions, the rest of c…

8d876ec

…onfig file is skipped.

ThomasAdam self-assigned this Jun 28, 2020

ThomasAdam added the type:bug Something's broken! label Jun 28, 2020

added helper function extract_monitor_config to extract monitor param…

6a79710

…s from string

ThomasAdam requested changes Jun 29, 2020

View reviewed changes

d-e-e-p and others added 2 commits June 29, 2020 02:26

Update modules/FvwmPager/FvwmPager.c

4cd9591

Co-authored-by: Thomas Adam <[email protected]>

fixed indentation and style..

ab33887

d-e-e-p added 2 commits June 29, 2020 23:11

if monitor name happens to get updated in ParseOptions, the rest of c…

bc76dec

…onfig file is skipped. also added helper function extract_monitor_config to extract monitor params from string

Merge branch 'master' of https://github.com/d-e-e-p/fvwm3

7b49d4f

ThomasAdam merged commit 086e1ac into fvwmorg:master Jun 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

if monitor name happens to get updated in ParseOptions, the rest of FvwmPager config is skipped #146

if monitor name happens to get updated in ParseOptions, the rest of FvwmPager config is skipped #146

d-e-e-p commented Jun 28, 2020

ThomasAdam commented Jun 28, 2020 •

edited

Loading

ThomasAdam left a comment

ThomasAdam Jun 29, 2020

ThomasAdam Jun 29, 2020

ThomasAdam Jun 29, 2020

ThomasAdam Jun 29, 2020

ThomasAdam Jun 29, 2020

ThomasAdam Jun 29, 2020

ThomasAdam Jun 29, 2020

d-e-e-p commented Jun 29, 2020 •

edited

Loading

ThomasAdam commented Jun 29, 2020

	void extract_monitor_config(struct fpmonitor m, char tline) {
	void extract_monitor_config(struct fpmonitor m, char tline)
	{

	extract_monitor_config(m,tline);
	extract_monitor_config(m, tline);

	extract_monitor_config(m2,next);
	extract_monitor_config(m2, next);

	void extract_monitor_config(struct fpmonitor m, char tline);
	static void extract_monitor_config(struct fpmonitor , char );

if monitor name happens to get updated in ParseOptions, the rest of FvwmPager config is skipped #146

if monitor name happens to get updated in ParseOptions, the rest of FvwmPager config is skipped #146

Conversation

d-e-e-p commented Jun 28, 2020

ThomasAdam commented Jun 28, 2020 • edited Loading

ThomasAdam left a comment

Choose a reason for hiding this comment

ThomasAdam Jun 29, 2020

Choose a reason for hiding this comment

ThomasAdam Jun 29, 2020

Choose a reason for hiding this comment

ThomasAdam Jun 29, 2020

Choose a reason for hiding this comment

ThomasAdam Jun 29, 2020

Choose a reason for hiding this comment

ThomasAdam Jun 29, 2020

Choose a reason for hiding this comment

ThomasAdam Jun 29, 2020

Choose a reason for hiding this comment

ThomasAdam Jun 29, 2020

Choose a reason for hiding this comment

d-e-e-p commented Jun 29, 2020 • edited Loading

ThomasAdam commented Jun 29, 2020

ThomasAdam commented Jun 28, 2020 •

edited

Loading

d-e-e-p commented Jun 29, 2020 •

edited

Loading