added configuration options for issue #585 (kill menu, client name in title) #587
Conversation
…name in title)
src/border.c
Outdated
| @@ -451,7 +451,17 @@ void DrawBorderHelper(const ClientNode *np) | |||
| if(np->name && np->name[0] && point.x < point.y) { | |||
| unsigned titleWidth = point.y - point.x; | |||
| const int sheight = GetStringHeight(FONT_BORDER); | |||
| const int textWidth = GetStringWidth(FONT_BORDER, np->name); | |||
| const int buffSize = (MAX_TITLE_LENGTH+1)*sizeof(char); | |||
| char *titleBuffer = calloc(MAX_TITLE_LENGTH+1,sizeof(char)); | |||
There was a problem hiding this comment.
What's the reason behind a fixed length allocation? I suspect 512 is probably enough for most reasonable use-cases, but then the sprintfs should probably be switched to snprintfs. I think I'd be more comfortable just allocating the right amount though, which is maybe a little error prone, but I think this would work:
char *titleBuffer;
if(settings.showMachineName) {
/* Space for 2 delimiters, space, terminator, and strings */
const size_t bufSize = strlen(np->name) + strlen(np->machineName) + 4;
titleBuffer = Allocate(bufSize);
sprintf(titleBuffer, "%s %c%s%c", np->name,
settings.machineNameDelimiters[0], np->machineName,
settings.machineNameDelimiters[1]);
} else {
titleBuffer = CopyString(np->name);
}
...
Release(titleBuffer);Also, JWM has macros to wrap malloc/realloc/free (Allocate/Reallocate/Release), which I try to use to make it easy to check for memory leaks and buffer overruns.
There was a problem hiding this comment.
I will make the suggested changes and remove the MAX_TITLE_LENGTH define
| @@ -1257,7 +1269,7 @@ respectively. | |||
| .B "TASK LIST STYLE" | |||
| .RS | |||
| The \fBTaskListStyle\fP tag controls the look of task lists. | |||
| The following attributea are supported: | |||
| The following attributes are supported: | |||
There was a problem hiding this comment.
Thanks for fixing all these typos!
jwm.1.in
Outdated
| .P | ||
| .B decorations | ||
| .RS | ||
| The window decorations to use. Valid options are \fBflat\fP and | ||
| \fBmotif\fP. \fBflat\fP is the default. | ||
| .RE | ||
| .P | ||
| .B machinename |
There was a problem hiding this comment.
I'm kind of conflicted on the name. As is it sounds like you're setting the machine name. Even the code has showMachineName, but that's a little on the long side. Should it be named showmachine instead?
There was a problem hiding this comment.
I like showmachine better - will make the change
There was a problem hiding this comment.
actually, I think the attribute should be showclient since it is based on the WM_CLIENT_MACHINE property.
jwm.1.in
Outdated
| @@ -1279,6 +1291,13 @@ Possible values are \fBdesktop\fP and \fBall\fP. The default | |||
| is \fBdesktop\fP. | |||
| .RE | |||
| .P | |||
| .B killmenu | |||
src/parse.c
Outdated
| temp = FindAttribute(tp->attributes, MN_DELIMITERS_ATTRIBUTE); | ||
| if(temp && strlen(temp)>=2) { | ||
| char *delims = calloc(strlen(temp)+1,sizeof(char)); | ||
| sprintf(delims,"%s",temp); |
There was a problem hiding this comment.
I think I would actually just declare settings.machineNameDelimiters as char machineNameDelimiters[2]; and just memcpy two characters here
|
Looks great! Thanks! |
|
Thank you for adding these changes. Now I (hopefully) won't have to work with Openbox or Fluxbox any more... |
|
@joewing Can you make a new release with this commit? It's been a while since previous one :) Thanks! |
|
Sure! I'll try to push out a release today. |
Please see comments in issue #585