nicklan / pnmixer

Volume mixer for the system tray
GNU General Public License v3.0
152 stars 32 forks source link

Fix indenting/coding style #96

Closed hasufell closed 9 years ago

hasufell commented 9 years ago
elboulangero commented 9 years ago

fixed 80char linewidth manually

The linewidth is given with the option -l100. If not specified, default is 80, so you can just remove the -l option.

I find it useful to use a slightly longer linewidth, especially when using glib, which can have some very long function names. But I'm not so sure, maybe it's better to stick to the usual 80 width, and go longer when it's really needed.

elboulangero commented 9 years ago

Maybe we can also setup some git hooks to prevent bad indentation before committing. I aleady saw that on a project.

hasufell commented 9 years ago

not sure if github lets you do real hooks, but automated style-check should work somehow

elboulangero commented 9 years ago

Wait wait wait !!!

I just stumbled on that: http://astyle.sourceforge.net/ I'll give it a try, I've never been so happy with indent. Maybe its output is better.

hasufell commented 9 years ago

I looked over every file manually, was fine. I can revert however if you want (was an accident push anyway, switched to the PR branch when the commit was already on master :o).

elboulangero commented 9 years ago

No problem, just give me some time to see if this astyle tool is better than indent. There are a few things that I don't like too much with indent, for example this bad habit to put spaces after pointers when the type is not a native C type: hide_me(GtkWidget * widget, GdkEvent * event, gpointer user_data) While a native C type gives: main(int argc, char *argv[])

elboulangero commented 9 years ago

Just pushed changes with astyle. Actually I find it smarter than indent. If you OK we can merge that to main, and I'm done with the indenting question for a while :)

hasufell commented 9 years ago

I merged it locally with git merge -X theirs ... (while being on the master branch). Looks ok. Should I push?

elboulangero commented 9 years ago

Yep. Are we done for the release ?

hasufell commented 9 years ago

We'll probably find the first bug after it has been released ;)

hasufell commented 9 years ago

I also want to see if I can fix the travis build so it builds gtk2 (with and without libnotify) as well.

hasufell commented 9 years ago

po/pnmixer.pot is removed on every compile (and recorded as unstaged change), that is a bit annoying

elboulangero commented 9 years ago

Yep, it's good to version it, so that translators can get the file easily, no need to type any command. But it disappears on every make cleancommand. I think an easy way is to patch the Makefile.in.in (provided by intltool) so that it doesn't touch pnmixer.pot anymore.

hasufell commented 9 years ago

Yep, it's good to version it, so that translators can get the file easily, no need to type any command. But it disappears on every make cleancommand. I think an easy way is to patch the Makefile.in.in (provided by intltool) so that it doesn't touch pnmixer.pot anymore.

Hm, I don't have a strong opinion there, but make -C po pnmixer.pot is pretty easy, no?

Also: I just ran make pnmixer.pot && make update-po which shows that the po-files are already slightly out of date after our indentation fixes.

elboulangero commented 9 years ago

Hm, I don't have a strong opinion there, but cd po && make pnmixer.pot is pretty easy, no?

I agree, but Benno from the TP insisted that translators shouldn't type any command to get the pot file. I don't have any opinion on that either, it depends on their workflow. Another option is: we do not version it, but then when we want the translators to work on PNMixer, we have to create explicitely an archive (with make dist), and tells the autotools that the archive must contain the pot file. Maybe it's a better approach.

elboulangero commented 9 years ago

Done, I added a dist hook to include the pot file in the dist tarball.

elboulangero commented 9 years ago

OK, I fixed the fuzzy translation, updated the pot files. Done with that. These pot files make my git statistics explode :D

hasufell commented 9 years ago

travis now builds all permutations ac62dbda8b5864cef6d9b3e6c6c32ebed6827f77