Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Too few areas available by default #133

Closed
rr- opened this issue Jun 5, 2015 · 13 comments
Closed

Too few areas available by default #133

rr- opened this issue Jun 5, 2015 · 13 comments

Comments

@rr-
Copy link

rr- commented Jun 5, 2015

After starting serious tinkering, lemonbar gave me tons of "astack overflow" errors. The default number of maximum 20 clickable areas is too little: consider 10 workspaces with actions for left click and right click - there's no room left for anything else...

Increasing N to 100 would relief the necessity to compile lemonbar ourselves and wouldn't impact memory usage at all.

@Chrysostomus
Copy link

I second the opinion. Just 5 workspaces with scroll support takes 15 clickable areas. Volumeicon takes further 4. 20 areas is really little.

@Slabity
Copy link

Slabity commented Jun 7, 2015

Should it even be constant at all? I wouldn't mind for the user to be able to specify the number of sections, or even dynamically allocate them as necessary.

@ghost
Copy link

ghost commented Sep 12, 2015

It's a stack, right? Have a linked list! Also, surely only the nesting should be stored? So, left click and right click would only be 2 slots? Not sure, haven't really read code. Any opinions on this?

@otommod
Copy link
Contributor

otommod commented Sep 12, 2015

While dynamic memory allocation (or a linked list) would allow for an arbitrary number of areas, it would complicate the code.

Since less is more, I think the "better" solution is to just raise N to 100.

@ghost
Copy link

ghost commented Sep 13, 2015

A linked list wouldn't complicate the code too much at all. Replace the slot member with head and tail. Adjust the iterations and the push, pop code. Et voila. But still, #define N 100 is a good solution

@ghost
Copy link

ghost commented Sep 14, 2015

Anyway, I don't care as area_t is 32B on my system (YMMV), so #define N 200 yields 6.25KiB, IOW 5.625KiB more than #define N 20. Simple.

@Ferdi265
Copy link

Because I had some time on my hands, I made a quick and dirty fork of lemonbar that implements area_stack_t with a dynamically allocated (in the sense that it isn't a compile time config) length for slots.

In my fork, invoking lemonbar with -S <number-here> will raise the limit to <number-here>. The changes are trivial, but as I said, it was a quick and dirty solution, so the code is not quite clean and nice (for example I just used atoi() for the integer parsing for the parameter).

Here's the fork: Repo

@ghost
Copy link

ghost commented Oct 12, 2015

You used strtoul() instead of atoi() 😄. IMHO the code is pretty clean

@Ferdi265
Copy link

That's because I cleaned it up immediately after writing this 😄

@LemonBoy
Copy link
Owner

Cool, mind submitting a PR ?

@ghost
Copy link

ghost commented Oct 24, 2015

The problem would be that it is based on krypt-n's which causes it to yield some extra commits (e.g. merge commits)

@LemonBoy
Copy link
Owner

Please check out the latest commit.

@Ferdi265
Copy link

Latest commit looks fine. I don't mind my fork not being merged in as long as the issue is fixed.

My fork will exist a while longer until krypt-n merged the changes into his xft-branch, which I lack the knowledge to merge correctly, so that it doesn't segfault.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants