1) There is no need for both the LOG (line 74) and LOG_DEBUG (line 82) functions, these can easily be condensed to one function
2) In the function TextWidth (line 101), if you are going to insist on using strlen(), there is no reason to actually do strlen() >= 1. Just do strlen() as it will return something >= 0.
I'll work on your sighandler.
]]>As for the next_window issue, I would suggest that if it fails to finding any windows it should just return the current window. I'll take another glance through the source in a little bit as well.
[e] I guess since they are non-standard functions (popen & pclose) you aren't really guarenteed to have them in stdio.h. You may need to grep your headers and see how your header is set up.
]]>// true = next, false = previous
int get_window(bool nex_prev)
{
int x = get_position(selected);
nex_prev ? x++ : x--;
while ( x >= 0 && x < max_windows) {
if(windows_container[x] != None)
{
LOG_DEBUG("Found next window at: %d\n", x);
return x;
}
nex_prev ? x++ : x--;
}
return -1;
}
Make sure you are including signal.h, that should eliminate the errors with your sighandler.
Including stdio.h should eliminate your problems with pclose and popen.
Try that stuff and post if you are still having problems.
[e] K&R is a good choice.
]]>extern FILE *popen (__const char *__command, __const char *__modes) __wur;
extern int pclose (FILE *__stream);
Temporary removed the sighandler part, so now all there's left are the errors about your get_window function. I guess I'll get into it tomorrow morning.
[edit] ok added stdbool.h and it's all fine
ターミナル (bare-rewrite-03) $ make
cc -Os -Wall -std=c99 -I/usr/include -L/usr/lib -lX11 -o barewm barewm.c
barewm.c:53: error: expected ')' before 'nex_prev'
barewm.c: In function 'sighandler':
barewm.c:90: error: 'WAIT_ANY' undeclared (first use in this function)
barewm.c:90: error: (Each undeclared identifier is reported only once
barewm.c:90: error: for each function it appears in.)
barewm.c: At top level:
barewm.c:150: error: expected ')' before 'nex_prev'
barewm.c: In function 'echo_output':
barewm.c:202: warning: implicit declaration of function 'popen'
barewm.c:202: warning: initialization makes pointer from integer without a cast
barewm.c:211: warning: implicit declaration of function 'pclose'
barewm.c: In function 'main':
barewm.c:570: error: storage size of 'act' isn't known
barewm.c:613: warning: implicit declaration of function 'sigemptyset'
barewm.c:614: error: 'SA_NOCLDSTOP' undeclared (first use in this function)
barewm.c:614: error: 'SA_RESTART' undeclared (first use in this function)
barewm.c:615: warning: implicit declaration of function 'sigaction'
barewm.c:570: warning: unused variable 'act'
make: *** [all] Error 1
ターミナル (bare-rewrite-03) $
max_title = strlen(title);
Really your C just needs touching up, perhaps read a good book.
[e] As far as declaring _most_ functions as int goes, that's alright for debugging. But in releases you should change functions that don't return things to void.
]]>PS: Welcome to the forums
The max_title part is just a variable that I add to in case I hit a title that's longer than the current value. With that I build my window's size
As per my lament here http://bbs.archlinux.org/viewtopic.php?id=81868, I can see that my C skills are really poor. I knew I came to the right conclusions there
Regarding my definition of most functions as int, I usually do that in case I need to check a succesfull run of the function. Not always bad I guess.
]]>1) It's generally better form to put all macros in all caps, and refrain from doing so to functions and other things that aren't macros. This is to help other people out who are reading your code.
2) On line 110 (TextWidth) of your code you do this, "if(strlen(text) >= 1 && text != NULL)", but that doesn't make any sense. If you want to avoid the exception raised from doing strlen(NULL) you need to switch the order inside the if statement to "if(text != NULL && strlen(text) >= 1)", but really what you should do is this "if(text && text[0])".
3) The function on line 117 (TextHeight) may be better served as a macro.
4) You can also clean the function on line 121 (name2color) up by doing this.
unsigned long name2color(const char *cid)
{
XColor tmpcol;
if(XParseColor(display, BARE_colormap, cid, &tmpcol)) {
if(XAllocColor(display, BARE_colormap, &tmpcol))
return tmpcol.pixel;
}
LOG("Cannot allocate \"%s\" color. Defaulting to black!\n", cid);
return BlackPixel(display, XDefaultScreen(display));
}
5) The function at line 137 (init_gc) should just be type void.
6) At line 157 (in get_prev_window), assuming that you are using C99 like a sane person, you can declare variables in your for statement (This applies throughout your code).
ex,
for (int i = 0; i < 10; i++)
7) As far as the functions at 154 (get_next_window) and 168 (get_prev_window), you should be able to combine them into one function, something like this.
// true = next, false = previous
int get_window(bool nex_prev)
{
int x = get_position(selected)
nex_prev ? x++ : x--;
while ( x >= 0 && x < max_windows) {
if(windows_container[x] != None)
{
LOG_DEBUG("Found next window at: %d\n", x);
return x;
}
nex_prev ? x++ : x--;
}
return -1;
}
8) In your function at 181 (grab_keyboard), I can't really say that the way you have it is bad. But personally I prefer explicit declarations when there are no parameters to a function so... int whatever(void) as opposed to int whatever(). The same applies throughout your code.
9) The function at 186 (echo_output) is a good example of a function that lacks error checking. In general you lack a lot of error checking, which you should fix.
10) The function at 206 (handle_keypress_event) also does no error checking, add it.
11) On line 213 (in handle_key_press_event) you are relying on the fact that whatever charset is in use will have numbers listed sequentially. Use isnum() instead.
Bad = if (key >= '0' && key <= '9')
12) In general you should focus more on dynamic memory allocation as opposed to static memory. So heap in favor of stack basically.
13) Line 339 (in list_windows) makes me wonder what the purpose of max_title is.
if(strlen(title) > max_title) max_title = strlen(title);
If you are just going to adjust the max title whenever a longer title occurs what's the point?
14) Code in the function on line 315 (list_windows) seems redundant, like some shorter function(s) could be reused in it to clean it up. I don't have time to really figure that out for sure right now though.
15) There is no need to have functions at line 428 (get_free_position) and line 441 (get_position) and line 455 (free_position) as they all do almost the same thing.
16) Last thing I noticed is that you are very inconsistent with naming conventions for functions and variables. You should sort this out, it will make your code much more pleasent to read.
Alright, those are the things that I noticed in about ~30 mins of scanning, however I need to get ready for class. Feel free to ask about anything, either how to implement it, or if what I said was just wacky.
]]>/me = Wra!th
Oh, thought so, just wasn't sure.
well the conf.h file is in the barewm branch, I suck at PKGBUILDs so I asked wonder to make one for me. I'm not that good with AUR, but I would imagine you could edit something before it gets built? I'll look into this later
No need to do this. Just make a new folder (i.e. $HOME/.barewm) with all the necessary files for compilation. Here's an excerpt from the Wiki how it's done with dwm:
Browse to the dwm source code directory you saved during the installation process (~/dwm in the example). The config.h found within this directory is where the general dwm preferences are stored.
[...]
Once your changes have been made, edit the PKGBUILD file in the same directory and delete the contents of the md5sums array so that it looks like this:md5sums=()
This will eliminate a checksum mismatch between the official config.h and your revised copy.
Now to compile and reinstall:
$ makepkg -efi
Assuming your changes were valid, this command will compile dwm, build and reinstall the resulting package (see makepkg -h for details).
[...]
edit:
btw, dumb question: where is the conf.h file?
Ooooh, yeah you are left with cycling them afaik. Too bad xmonad doesn't have the option to output windows to stdout like ratpoison (IN YOUR FACE XMONAD!:D), you could write a script with dmenu or ratmenu to get something similar.
Oh Thanks that was really helpfull (Now I need someone to help)
]]>