search

dannyedel / dspdfviewer

Dual-Screen PDF Viewer for latex-beamer
http://dspdfviewer.danny-edel.de
GNU General Public License v2.0
219 stars 29 forks source link

Add message box with keybindings bound to '?' key. #29

Closed zakkak closed 9 years ago

zakkak commented 9 years ago

messagebox

dannyedel commented 9 years ago

Thanks, this is a really good idea and its very nice to do it in Pull-Request form : )

I'll be reading it through now, and either merge or give comments at the points in code. One thing I see though: There's a lot of whitespace changes, which makes it a bit hard to see the actual changes.

While that is obviously my fault for commiting such.. uhm.. things (trailing whitespace, blank lines at eof etc...) I'm a bit disappointed that kdevelop/kate did not prevent me from doing that in the first place. (Or I have not found the correct configuration)

I've read up and the github blog says that by appending w=1 we can filter whitespace changes while viewing.

Side question: Do you know a good tool/editor to quickly walk through all those files, fix whitespace and commit? Maybe worth it's own pullrequest "fix whitespace"... I'll get on to reading the actual code now.

dannyedel commented 9 years ago

Found two wording issues, otherwise cool!

zakkak commented 9 years ago

Sorry about the whitespace changes, it is my editor setup that removes trailing spaces.

Another "annoying" IMO thing is that indentation is not consistent. The code is indented using spaces and whenever a group of spaces can be reduced to a tab it gets replaced. I suggest either using solely tabs or spaces.

I am actually a fan of indenting with tabs and aligning with spaces, there is a nice example at the emacswiki.

smarttabs

zakkak commented 9 years ago

Regarding your side question. Both vim end emacs have options to remove trailing spaces, I do not remember if kate was capable of doing the same, if not there is probably some plugin.

Another approach is the use of tools like uncrustify that apart from removing whitespace allow for a consistent code style. Unfortunately though, it is a pain to configure to your needs and is not available in every distribution (you most likely need to compile from source).

Also find . -name ".git" -prune -o -type f -exec sed -i 's/ *$//g' {} \; should do the trick.

dannyedel commented 9 years ago

Thanks for this function! Merge on the way. Whitespace-issue redirected to #30