Closed GoogleCodeExporter closed 9 years ago
I also wanted to output some stats on the number of such violations (Though
many of
these will be due to the license header at the top, which is 10 lines:
[teju@vimzard mingus]$ grep "^ " -c -R --include="*.py" .
./doc/generate_wiki_docs.py:10
./unittest/test_progressions.py:0
./unittest/test_fluidsynth.py:2
./unittest/run_tests.py:12
./unittest/test_Composition.py:0
./unittest/test_diatonic.py:1
./unittest/test_Bar.py:0
./unittest/test_notes.py:0
./unittest/run_lilypond_tests.py:10
./unittest/test_value.py:0
./unittest/test_NoteContainers.py:42
./unittest/test_chords.py:7
./unittest/run_fluidsynth_tests.py:10
./unittest/test_Note.py:19
./unittest/test_meter.py:0
./unittest/test_LilyPond.py:0
./unittest/test_tunings.py:37
./unittest/test_Track.py:0
./unittest/test_fft.py:7
./unittest/test_intervals.py:32
./unittest/test_Instrument.py:0
./unittest/test_scales.py:0
./unittest/test_tablature.py:2
./unittest/test_Suite.py:0
./mingus_examples/pygame-piano/pygame-piano.py:0
./mingus_examples/improviser/improviser.py:0
./mingus_examples/pygame-drum/pygame-drum.py:0
./mingus_examples/play_progression/play-progression.py:0
./googlecode_upload.py:172
./setup.py:31
./mingus/midi/fluidsynth.py:96
./mingus/midi/MidiTrack.py:10
./mingus/midi/MidiEvents.py:0
./mingus/midi/__init__.py:10
./mingus/midi/Sequencer.py:112
./mingus/midi/MidiFileOut.py:12
./mingus/midi/pyFluidSynth.py:181
./mingus/midi/SequencerObserver.py:77
./mingus/midi/MidiFileIn.py:12
./mingus/extra/MusicXML.py:171
./mingus/extra/LilyPond.py:18
./mingus/extra/__init__.py:10
./mingus/extra/tunings.py:364
./mingus/extra/fft.py:113
./mingus/extra/tablature.py:331
./mingus/__init__.py:10
./mingus/core/progressions.py:10
./mingus/core/chords.py:11
./mingus/core/__init__.py:10
./mingus/core/scales.py:10
./mingus/core/mt_exceptions.py:12
./mingus/core/value.py:10
./mingus/core/intervals.py:20
./mingus/core/notes.py:10
./mingus/core/diatonic.py:12
./mingus/core/meter.py:10
./mingus/containers/Bar.py:16
./mingus/containers/NoteContainer.py:69
./mingus/containers/Instrument.py:11
./mingus/containers/__init__.py:10
./mingus/containers/mt_exceptions.py:10
./mingus/containers/Track.py:10
./mingus/containers/Suite.py:15
./mingus/containers/Note.py:63
./mingus/containers/Composition.py:10
Original comment by arunchag...@gmail.com
on 25 Nov 2009 at 6:16
Yes, the indentation is messy and should be changed or at least fixed. I'd
personally
like to switch to 4 spaces if we are going through all the files, to avoid
confusion
and to comply to PEP-8. This would probably involve some more manual work
though so
let me know what you think.
Original comment by Rhijnauwen@gmail.com
on 25 Nov 2009 at 6:26
Hey,
I've looked around for a script that would do the job, and found PythonTidy. I've
attached the script + a bash script that should take care of rectifying all the
code.
I've tested it with the unittests, and they pass fine.
Also, I'd like to ask how I should go about committing my changes, and attaching
it to the issue tracker.
Cheers,
Arun.
Original comment by arunchag...@gmail.com
on 26 Nov 2009 at 12:22
Attachments:
Please use this tidy-all.sh instead - it preserves the file permissions.
Original comment by arunchag...@gmail.com
on 26 Nov 2009 at 12:25
Attachments:
Nice work! The script does seem to fix all indentation errors, but
unfortunately it
also messes with the docstrings, which are used by the (reference) documentation
generator. Specifically, it removes the trailing backslashes and inserts
indentation
and this breaks the formatting of the code examples and explanations embedded
in the
strings. The doc generator would be hard to change, because it wouldn't know
when to
insert and remove line breaks and spaces; so unfortunately this means we need to
leave the docstrings alone.
It seems that the offending code in PythonTidy is the put_doc method beginning
on
line 1706. This should just output the docstring as is. Setting
LEFTJUST_DOC_STRINGS
and WRAP_DOC_STRINGS to False fixes the line break issue, but still removes the
pending slash character (I think it would have to parse the file itself to be
able to
retain it, because it's manipulating the AST now). So this is still not really
an option.
What we need in put_doc is something that does this: get the docstring as
unicode,
split lines, wrap the lines that are too long (>80 characters) by inserting a
backslash and a linebreak after a space and leave the rest alone (no indenting,
etc).
> Also, I'd like to ask how I should go about committing my changes, and
attaching
it to the issue tracker.
For example, if this is fixed you could just
svn ci -m "Replaced tabs with spaces in every module. Closes issue 91."
This will automatically close the issue and send some mails to the dev list:
http://groups.google.com/group/mingus-devel
See also
http://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_cont
rol
for more info on tracker integration.
Hope this helps,
Bart
Original comment by Rhijnauwen@gmail.com
on 26 Nov 2009 at 7:58
I've changed the way WRAP_DOC_STRINGS works and this seems to do the right
thing. See
the patch.
Original comment by Rhijnauwen@gmail.com
on 26 Nov 2009 at 8:19
Attachments:
Disregard that...still not wrapping correctly.
Original comment by Rhijnauwen@gmail.com
on 26 Nov 2009 at 8:23
This patch does the right thing for function docstrings, but the consequence is
that
it now also messes with the header, because it can't differentiate between the
two.
A (slightly hacky) solution to this would be to check for the header in put_doc
(a
newline and 80 ='s) and leave that alone in its entirety. I might code this up
later,
because I don't know what else we could do — except for using ggVG= in vim
for every
file, but this is kind of tedious and not as maintainable as a script that we
can use
as hook.
Original comment by Rhijnauwen@gmail.com
on 26 Nov 2009 at 8:45
Attachments:
Here is my last patch against the PythonTidy-1.19 file you attached. This takes
care
of the header strings as well. Hopefully this will cover all the cases, but I'm
not
100% certain of that (I'm not sure if docstrings have been used in other
manners).
Let me know what you think.
Original comment by Rhijnauwen@gmail.com
on 26 Nov 2009 at 12:04
Attachments:
Hey,
Ah, sorry, I didn't check for the example code formatting. The script's
modifications work great. I just changed one line to make it a little less
hacky - I
dropped the "="*80 and made the COL_LIMIT=80. The output should be the same,
and does
seem to be that way as well.
Cheers,
Arun.
Original comment by arunchag...@gmail.com
on 26 Nov 2009 at 1:23
Hey,
Ah, sorry, I didn't check for the example code formatting. The script's
modifications work great. I just changed one line to make it a little less
hacky - I
dropped the "="*80 and made the COL_LIMIT=80. The output should be the same,
and does
seem to be that way as well.
Cheers,
Arun.
Original comment by arunchag...@gmail.com
on 26 Nov 2009 at 1:26
Attachments:
Thanks that works nicely. This means the whole if statement can now also be
removed.
See my patch.
That should do it, I think. You can now use your tidy-all script and commit the
changes if you want.
Original comment by Rhijnauwen@gmail.com
on 26 Nov 2009 at 2:06
Attachments:
Closed by http://code.google.com/p/mingus/source/detail?r=579.
(You added issue 90 instead of 91 to the commit message.)
Original comment by Rhijnauwen@gmail.com
on 26 Nov 2009 at 2:35
Original issue reported on code.google.com by
arunchag...@gmail.com
on 25 Nov 2009 at 6:08