Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Tue, 05 Jul 2011 00:18:21 -0700, Trevor Daniels t.dani...@treda.co.uk wrote: Carl Sorensen wrote Tuesday, July 05, 2011 7:12 AM long_variable_name = (first_term + second_term); I prefer this indentation too. If Emacs users forget the brackets Astyle will indent it, but without the brackets. Can Astyle/fixcc be made to add brackets to keep Emacs sweet? Astyle has no capability to add a pair of () , and I think that's too much to ask of the regular expression filter in fixcc. We humans could take care of it ourselves. Emacs has a function c-lineup-assignments that aligns to the =, even without the extra (). I tried it in emacs and it works great. However, I do not know Lisp well enough to configure the call from fixcc.py properly so that emacs indents with c-lineup-assignments. - Keith ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 7/6/11 1:43 PM, Keith OHara k-ohara5...@oco.net wrote: On Tue, 05 Jul 2011 00:18:21 -0700, Trevor Daniels t.dani...@treda.co.uk wrote: Carl Sorensen wrote Tuesday, July 05, 2011 7:12 AM long_variable_name = (first_term + second_term); I prefer this indentation too. If Emacs users forget the brackets Astyle will indent it, but without the brackets. Can Astyle/fixcc be made to add brackets to keep Emacs sweet? Astyle has no capability to add a pair of () , and I think that's too much to ask of the regular expression filter in fixcc. We humans could take care of it ourselves. Emacs has a function c-lineup-assignments that aligns to the =, even without the extra (). I tried it in emacs and it works great. However, I do not know Lisp well enough to configure the call from fixcc.py properly so that emacs indents with c-lineup-assignments. If emacs *can* do it the way we want it, and astyle *will* do it the way we want it, then I think we ought to just go ahead with astyle, and not worry if an emacs-style reindent gets through occasionally. After all, we can fix it with fixcc/astyle. If we had part of the release process be running fixcc/astyle, we'd have our code continually cleaned up. Just my $0.02 Thanks, Carl - Keith ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Carl Sorensen wrote Wednesday, July 06, 2011 11:02 PM If emacs *can* do it the way we want it, and astyle *will* do it the way we want it, then I think we ought to just go ahead with astyle, and not worry if an emacs-style reindent gets through occasionally. After all, we can fix it with fixcc/astyle. +1 If we had part of the release process be running fixcc/astyle, we'd have our code continually cleaned up. +1 This shouldn't be too difficult to achieve. Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 7/4/11 11:32 PM, Jan Nieuwenhuizen jann...@gnu.org wrote: Also, I tried to make the output of the modified fixcc+asytle pass unchanged through emacs, but the very many cases of line-broken asssignments will be different. That's a problem. long_variable_name = first_term + second_term; // emacs This is correct. If we want to change the indentation, can't we do it with long_variable_name = (first_term + second_term); and have Emacs indent it this way? Emacs users will forget to run fixcc That won't do, sorry. Let's make it as easy as possible for non-Emacs users; but choosing a coding style that Emacs does not support is a no-go. This seems reasonable. We can still opt for using the current astyle solution, just as long as we see that this approach is a huge step forward and recognise that astyle still needs some fixes (and request these on the astyle development list). This also seems reasonable. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Carl Sorensen writes: If we want to change the indentation, can't we do it with long_variable_name = (first_term + second_term); and have Emacs indent it this way? We could; python requires this anyway. I still prefer option 3 below: have astyle support 2 space indentation for broken statements, but this is nit-picking. This seems reasonable. great, thanks Jan -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Carl Sorensen wrote Tuesday, July 05, 2011 7:12 AM On 7/4/11 11:32 PM, Jan Nieuwenhuizen jann...@gnu.org wrote: Also, I tried to make the output of the modified fixcc+asytle pass unchanged through emacs, but the very many cases of line-broken asssignments will be different. That's a problem. long_variable_name = first_term + second_term; // emacs This is correct. If we want to change the indentation, can't we do it with long_variable_name = (first_term + second_term); and have Emacs indent it this way? I prefer this indentation too. If Emacs users forget the brackets Astyle will indent it, but without the brackets. Can Astyle/fixcc be made to add brackets to keep Emacs sweet? Emacs users will forget to run fixcc That won't do, sorry. Let's make it as easy as possible for non-Emacs users; but choosing a coding style that Emacs does not support is a no-go. This seems reasonable. Yes. We can still opt for using the current astyle solution, just as long as we see that this approach is a huge step forward and recognise that astyle still needs some fixes (and request these on the astyle development list). This also seems reasonable. Also yes. Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 7/4/11 11:32 PM, Jan Nieuwenhuizen janneke at gnu.org wrote: Also, I tried to make the output of the modified fixcc+asytle pass unchanged through emacs, but the very many cases of line-broken asssignments will be different. Emacs users will forget to run fixcc That won't do, sorry. Let's make it as easy as possible for non-Emacs users; but choosing a coding style that Emacs does not support is a no-go. Well, the good news is + Most of the problems we had were due to the prefilter, not emacs + emacs onlyd does the indentation, so probably we need not worry about which emacs version + emacs can indent spaces instead of tabs + The prefilter can protect block comments from emacs indentation Therefore here is a new patch set showing a restricted pre-filter using emacs. I have only had a quick look at the diffs but what I saw looked good. Runnign the formatter on the full tree gives a clean make; I have a busy Tuesday, but at the end of it I can run make check and report back here. Since the operation is emacs( prefilter (code)), then further editing using emacs will make no changes -- assuming emacs indentation is idempotent, which is my big word for the day. http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py File scripts/auxiliar/fixcc.py (right): http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py#newcode64 scripts/auxiliar/fixcc.py:64: # trailing operator, but don't un-trail assignment operators or close angle-braces On 2011/07/04 20:45:23, Keith wrote: Looking through the existing code, line-broken assignments /do/ get the '=' on the second line long_name = long_initializer; so I could restore that. Done. http://codereview.appspot.com/4662074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
k-ohara5...@oco.net wrote Monday, July 04, 2011 4:33 AM I really, really, wish Astyle had a fully gnu-compliant mode out of the box. I went through the regexp's again and think this is good to go. I agree. This is a big improvement, and would give us control over the layout style ourselves (rather than what emacs does). The odd niggle is not important; consistency is. Thanks for pursuing this to a satisfactory conclusion, Keith! Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Trevor Daniels writes: I agree. This is a big improvement, and would give us control over the layout style ourselves (rather than what emacs does). While the work being done here is possibly a good thing, let me remind you once more that we are a GNU project and thus use the GNU standards and thus we need no control over, we need not decide about, we need not discuss layout style. That's a feature. The GNU standards are implemented by Emacs, and if it makes an error, then that's a serious bug that should be reported (to emacs). It seems to me that someone is spending a lot of effort `just' to accomodate people who haven't found how awesome Emacs is to edit code and thus introduce layout problems. This makes it now easier to use another editor than Emacs, which may or may not be an improvement. While choice is good, in this case it decreases the need for non-Emacs users to try Emacs, and I'm not at all sure if that's a feature. Greetings, Jan. -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Mon, Jul 04, 2011 at 11:28:19AM +0200, Jan Nieuwenhuizen wrote: While the work being done here is possibly a good thing, let me remind you once more that we are a GNU project and thus use the GNU standards and thus we need no control over, we need not decide about, we need not discuss layout style. That's a feature. then WTF is fixcc.py ?! (the original one, pre-astyle) LilyPond has a *huge* history of deviating from GNU standards. This makes it now easier to use another editor than Emacs, which may or may not be an improvement. While choice is good, in this case it decreases the need for non-Emacs users to try Emacs, and I'm not at all sure if that's a feature. I'm sorry, I thought this was a project to create aesthetically beautiful notation. Not an emacs is awesome and let's force everybody to use my favorite text editor project. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Am Montag 04 Juli 2011, 11:28:19 schrieb Jan Nieuwenhuizen: The GNU standards are implemented by Emacs, and if it makes an error, then that's a serious bug that should be reported (to emacs). It seems to me that someone is spending a lot of effort `just' to accomodate people who haven't found how awesome Emacs is to edit code and thus introduce layout problems. This makes it now easier to use another editor than Emacs, which may or may not be an improvement. While choice is good, in this case it decreases the need for non-Emacs users to try Emacs, and I'm not at all sure if that's a feature. Oh that irony! Isn't one of the main ideas of FOSS to give you the freedom to choose which software you want to use rather than locking you down to one particular choice? It's really ironic that the argument now is that everyone is supposed to use emacs and those who don't should be slightly forced to use it. And no, I don't use emacs, and I won't use it in the future either (I'm using kate). If that means that my code is not formatted according to the GNU standard, okay so be it. What doesn't really help is that the GNU coding standard is not very clearly spelled out. Several things are defined as teh output of Emacs (compare the OOXML specification, in part defined as behave as Wort 97 does). Cheers, REInhold -- -- Reinhold Kainhofer, Vienna University of Technology, Austria email: reinh...@kainhofer.com, http://reinhold.kainhofer.com/ * Financial and Actuarial Mathematics, TU Wien, http://www.fam.tuwien.ac.at/ * Edition Kainhofer Music Publishing, http://www.edition-kainhofer.com/ * LilyPond music typesetting software, http://www.lilypond.org/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Jan Nieuwenhuizen wrote Monday, July 04, 2011 10:28 AM Trevor Daniels writes: I agree. This is a big improvement, and would give us control over the layout style ourselves (rather than what emacs does). While the work being done here is possibly a good thing, let me remind you once more that we are a GNU project and thus use the GNU standards and thus we need no control over, we need not decide about, we need not discuss layout style. That's a feature. I looked at the GNU coding standards for C: http://www.gnu.org/prep/standards/standards.html#Formatting They are defined very loosely, and are recommendations rather than hard requiements. To quote: We don’t think of these recommendations as requirements, because it causes no problems for users if two different programs have different formatting styles. But whatever style you use, please use it consistently, since a mixture of styles within one program tends to look ugly. If you are contributing changes to an existing program, please follow the style of that program. So what we are doing seems to me to be quite compatible with GNU standards. The GNU standards are implemented by Emacs, and if it makes an error, then that's a serious bug that should be reported (to emacs). It seems to me that someone is spending a lot of effort `just' to accomodate people who haven't found how awesome Emacs is to edit code and thus introduce layout problems. This makes it now easier to use another editor than Emacs, which may or may not be an improvement. While choice is good, in this case it decreases the need for non-Emacs users to try Emacs, and I'm not at all sure if that's a feature. Surely you're not serious! Are you?? Decreasing the need for Emacs is the whole point of this discussion. Greetings, Jan. Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 7/4/11 3:28 AM, Jan Nieuwenhuizen jann...@gnu.org wrote: Trevor Daniels writes: I agree. This is a big improvement, and would give us control over the layout style ourselves (rather than what emacs does). While the work being done here is possibly a good thing, let me remind you once more that we are a GNU project and thus use the GNU standards and thus we need no control over, we need not decide about, we need not discuss layout style. That's a feature. The GNU standards are implemented by Emacs, and if it makes an error, then that's a serious bug that should be reported (to emacs). It seems to me that someone is spending a lot of effort `just' to accomodate people who haven't found how awesome Emacs is to edit code and thus introduce layout problems. This makes it now easier to use another editor than Emacs, which may or may not be an improvement. While choice is good, in this case it decreases the need for non-Emacs users to try Emacs, and I'm not at all sure if that's a feature. I've tried Emacs, and I can't see the benefits justifying fighting through the learning curve. It's not comfortable for me; I have 30+ years of experience with vi(m), so it's hardwired in my bones. We need more, not fewer, developers on LilyPond. Why would we want to force people to try Emacs as a condition of helping out with LilyPond? Do we want to try to prohibit people from using Jedit, or Frescobaldi, since they're not Emacs? And I refuse, on principle, to accept a standard that says do it the way software package XYZ does it. This is not a standard, it's an excuse for not having a standard. That's behavior I'd expect from a proprietary software house, not from an organization that advocates software freedom. It's a peculiar type of freedom that says you're free to do anything you want with the software, except make changes in an editor other than our approved editor. Now I realize that nobody is forced to follow the GNU coding standards; they can modify the code to their heart's content and keep a separate distro from the official distro. But there's something philosophically wrong about this, and it seems to me that it's largely laziness on the part of the FSF.They are unwilling to define a standard that is other than machine-readable (a set of elisp macros and settings is only machine-readable, even if it's human understandable). So although this is a GNU project, I feel perfectly comfortable with advocating a style that is enforced by a project-specific tool that can be used by contributors working in any editor, rather than tying it to Emacs. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Mon, 04 Jul 2011 09:32:21 -0700, Trevor Daniels t.dani...@treda.co.uk wrote: Jan Nieuwenhuizen wrote Monday, July 04, 2011 10:28 AM It seems to me that someone is spending a lot of effort `just' to accommodate people who haven't found how awesome Emacs is to edit code and thus introduce layout problems. Surely you're not serious! Are you?? The emacs versus vi religious war is an old running joke. You'll notice that some of the gnu coding standards you linked to, like braces on do-while, are specifically designed to defend against emacs's awesome ability to introduce layout problems. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 2011/07/04 09:28:26, janneke_gnu.org wrote:
The GNU standards are implemented by Emacs,
Well, it looks like the problems with fixcc.py were caused by regexps,
not emacs.
Also, I tried to make the output of the modified fixcc+asytle pass
unchanged through emacs, but the very many cases of line-broken
asssignments will be different.
long_variable_name = first_term
+ second_term; // emacs
long_variable_name = first_term
+ second_term; // astyle
Emacs users will forget to run fixcc (because they are lazy heathens, as
all vi users know :-) causing distracting whitespace changes in the
diffs.
So *if* we can add back regexps to fixcc.py that do what neither emacs
nor programmers do automatically:
pad parentheses me-origin ();
unpad parenthesis-groups if ((a mask) == (d mask))
pad operators array[i + 1]
without breaking other things, then we could go back to call emacs for
the indenting.
Then hopefully the script would commute with emacs
fixcc.py (code) == emacs ( fixcc.py (code))
Patches welcome.
But until that is done, my proposal remains this shorter
prefilter+astyle.
http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py
File scripts/auxiliar/fixcc.py (left):
http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py#oldcode148
scripts/auxiliar/fixcc.py:148:
('((if|while)\s+\(([^\)]|\([^\)]*\))*\))\s*;', '\\1\n;'),
This is what was line-breaking the semicolon on an unbraced do-while!
do
something;
while (relevant)
;
So I was wrong to blame emacs for this one when I said
You'll notice that some of the gnu coding standards
you linked to, like braces on do-while, are
specifically designed to defend against emacs's
http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py
File scripts/auxiliar/fixcc.py (right):
http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py#newcode64
scripts/auxiliar/fixcc.py:64: # trailing operator, but don't un-trail
assignment operators or close angle-braces
Looking through the existing code, line-broken assignments /do/ get the
'=' on the second line
long_name
= long_initializer;
so I could restore that.
http://codereview.appspot.com/4662074/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Mon, Jul 04, 2011 at 09:58:14PM +0200, Jan Nieuwenhuizen wrote: Carl Sorensen writes: I've tried Emacs, and I can't see the benefits justifying fighting through the learning curve. It's not comfortable for me; I have 30+ years of experience with vi(m), so it's hardwired in my bones. Something went wrong here; it's awesome if we can manage not to depend on Emacs anymore. Ok, so you do not object us adopting fixcc-with-astyle as the official formatter for C++ code? I'll modify the actual GOP-PROP and move it to the (probable decision) 7-day countdown. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Also, I tried to make the output of the modified fixcc+asytle pass unchanged through emacs, but the very many cases of line-broken asssignments will be different. That's a problem. long_variable_name = first_term + second_term; // emacs This is correct. Emacs users will forget to run fixcc That won't do, sorry. Let's make it as easy as possible for non-Emacs users; but choosing a coding style that Emacs does not support is a no-go. We can still opt for using the current astyle solution, just as long as we see that this approach is a huge step forward and recognise that astyle still needs some fixes (and request these on the astyle development list). Greetings, Jan. -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Graham Percival writes: Ok, so you do not object us adopting fixcc-with-astyle as the official formatter for C++ code? It seems that this is a huge step forward; just don't change the definition to `whatever astyle can currently handle'; rather than: fixcc is okay and we're working with the astyle developers on some fixes. Jan -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 2011/07/03 02:41:15, Carl wrote: Can you make it possible for us to see the diff caused by applying this script to the files you've mentioned? I'll make a separate git branch showing the output once a few minor issues are done. Keith: 1. all // comments are now hard-left (i.e. in column 0). Is that intentional? it looks weird in the diff. 2. some multi-line comments are still modofied: diff --git a/lily/break-alignment-interface.cc b/lily/break-alignment-interface. index d368f83..0f08880 100644 --- a/lily/break-alignment-interface.cc +++ b/lily/break-alignment-interface.cc @@ -65,7 +65,7 @@ Break_alignment_interface::ordered_elements (Grob *grob) vectorGrob * writable_elts (elts); /* -Copy in order specified in BREAK-ALIGN-ORDER. + Copy in order specified in BREAK-ALIGN-ORDER. */ vectorGrob * new_elts; for (; scm_is_pair (order); order = scm_cdr (order)) #2 isn't important, but #1 looks like a bug. Thanks so much for working on this! Cheers, - Graham http://codereview.appspot.com/4662074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 2011/07/03 07:22:07, Graham Percival wrote: 1. all // comments are now hard-left (i.e. in column 0). This is fixed in the new patch set. The pre-filter was collapsing all whitespace, bringing single-line comments to the left edge, and astyle is conservative about re-indenting these, unless we change options. I just made the pre-filter less aggressive, because I hope the prefilter eventually withers away to nothing. 2. some multi-line comments are still modified: vectorGrob * writable_elts (elts); /* -Copy in order specified in BREAK-ALIGN-ORDER. + Copy in order specified in BREAK-ALIGN-ORDER. */ Astyle preserved the alignment, relative to the /*, in the original. I see comments being well-preserved -- re-indented as solid blocks with the contents intact -- if I compare the results to master. I'll push this fix to dev/gperciva-astyle, so we don't get confused between versions http://codereview.appspot.com/4662074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Sun, 03 Jul 2011 01:15:04 -0700, k-ohara5...@oco.net wrote: On 2011/07/03 07:22:07, Graham Percival wrote: 2. some multi-line comments are still modified: Astyle preserved the alignment, relative to the /*, in the original. That is to say, I think you were comparing against the output of fixcc.py+emacs Emacs re-formats the comments, so the mis-formatting you see was in the original. I prefer the way astyle leaves any (mis-) formatting inside block comments. On Sat, 02 Jul 2011 19:41:15 -0700, carl.d.soren...@gmail.com wrote: Can you make it possible for us to see the diff caused by applying this script to the files you've mentioned? I will soon (unless Graham beats me to it) for a small sampling of files. It was awkward to set up branches correctly to create the diff; I'll get back to it when I'm not tired. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Sun, Jul 03, 2011 at 01:43:40AM -0700, Keith OHara wrote: On Sun, 03 Jul 2011 01:15:04 -0700, k-ohara5...@oco.net wrote: Astyle preserved the alignment, relative to the /*, in the original. That is to say, I think you were comparing against the output of fixcc.py+emacs Emacs re-formats the comments, so the mis-formatting you see was in the original. mao, how many times am I going to make the same mistake?! On Sat, 02 Jul 2011 19:41:15 -0700, carl.d.soren...@gmail.com wrote: Can you make it possible for us to see the diff caused by applying this script to the files you've mentioned? I will soon (unless Graham beats me to it) for a small sampling of files. It was awkward to set up branches correctly to create the diff; I'll get back to it when I'm not tired. Here's a complete run: http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commit;h=710b0d99b331a824d0ed3d09f3f5f43d559b71b0 or on your own git: git diff origin/dev/gperciva-fixcc origin/dev/gperciva-astyle-keith I've spent a few minutes skimming through it without finding anything disagreeable. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 7/3/11 5:49 AM, Graham Percival gra...@percival-music.ca wrote:
On Sun, Jul 03, 2011 at 01:43:40AM -0700, Keith OHara wrote:
On Sat, 02 Jul 2011 19:41:15 -0700, carl.d.soren...@gmail.com wrote:
Can you make it possible for us to see the diff caused by applying this
script to the files you've mentioned?
I will soon (unless Graham beats me to it) for a small sampling of files.
It was awkward to set up branches correctly to create the diff;
I'll get back to it when I'm not tired.
Here's a complete run:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commit;h=710b0d99b331a824
d0ed3d09f3f5f43d559b71b0
Thanks, Graham!
Nearly every change I saw looked like an improvement.
There was, however, one change that I think is not good:
diff --git a/lily/book.cc b/lily/book.cc
@@ -73,9 +73,9 @@ Book::Book (Book const s)
SCM entry = scm_car (p);
if (Score *newscore = unsmob_score (entry))
-*t = scm_cons (newscore-clone ()-unprotect (), SCM_EOL);
+* t = scm_cons (newscore-clone ()-unprotect (), SCM_EOL);
else if (Page_marker *marker = unsmob_page_marker (entry))
-*t = scm_cons (marker-clone ()-unprotect (), SCM_EOL);
+* t = scm_cons (marker-clone ()-unprotect (), SCM_EOL);
else
{
/* This entry is a markup list */
Note that *t has changed to * t. I much prefer *t.
@@ -123,7 +123,7 @@ Book::mark_smob (SCM s)
}
int
-Book::print_smob (SCM, SCM p, scm_print_state*)
+Book::print_smob (SCM, SCM p, scm_print_state *)
{
scm_puts (#Book, p);
return 1;
Note that scm_print_state* has changed to scm_print_state *
Again, I prefer scm_print_state*
But I'd be willing to live with these changes if we could get a concrete and
stable definition of c++ style.
Thanks,
Carl
or on your own git:
git diff origin/dev/gperciva-fixcc origin/dev/gperciva-astyle-keith
I've spent a few minutes skimming through it without finding
anything disagreeable.
Cheers,
- Graham
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 3 July 2011 13:18, Carl Sorensen c_soren...@byu.edu wrote:
On 7/3/11 5:49 AM, Graham Percival gra...@percival-music.ca wrote:
On Sun, Jul 03, 2011 at 01:43:40AM -0700, Keith OHara wrote:
On Sat, 02 Jul 2011 19:41:15 -0700, carl.d.soren...@gmail.com wrote:
Can you make it possible for us to see the diff caused by applying this
script to the files you've mentioned?
I will soon (unless Graham beats me to it) for a small sampling of files.
It was awkward to set up branches correctly to create the diff;
I'll get back to it when I'm not tired.
Here's a complete run:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commit;h=710b0d99b331a824
d0ed3d09f3f5f43d559b71b0
Thanks, Graham!
Nearly every change I saw looked like an improvement.
+1
Looks pretty good to me.
@@ -123,7 +123,7 @@ Book::mark_smob (SCM s)
}
int
-Book::print_smob (SCM, SCM p, scm_print_state*)
+Book::print_smob (SCM, SCM p, scm_print_state *)
{
scm_puts (#Book, p);
return 1;
Note that scm_print_state* has changed to scm_print_state *
Again, I prefer scm_print_state*
Since we associate the pointer operator with variables rather than
types I'd argue
scm_print_state *
is more `correct', even for unused variables.
Cheers,
Neil
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 3 July 2011 12:49, Graham Percival gra...@percival-music.ca wrote:
I've spent a few minutes skimming through it without finding
anything disagreeable.
A few fishy looking changes:
int lily_cookie_fprintf (void *file, char const *format, ...)
-__attribute__ ((format (printf, 2, 3)));
+ __attribute__ ((format (printf, 2, 3)));
It looks like the function attribute isn't associated with the function above.
diff --git a/flower/include/matrix.hh b/flower/include/matrix.hh
index 6a9576c..4f8f7d4 100644 (file)
--- a/flower/include/matrix.hh
+++ b/flower/include/matrix.hh
@@ -22,8 +22,8 @@
#include std-vector.hh
-templateclass T, class A=std::allocatorT
-class Matrix
+template class T, class A = std::allocatorT
+ class Matrix
templates are a bit messed up
{
public:
MatrixT, A ()
@@ -32,7 +32,7 @@ public:
}
MatrixT, A (vsize rows, vsize columns, T const t)
- : data_(rows * columns, t)
+: data_ (rows *columns, t)
misinterpreted multiplication sign
diff --git a/flower/include/rational.hh b/flower/include/rational.hh
index bf01c8b..b0dd95b 100644 (file)
--- a/flower/include/rational.hh
+++ b/flower/include/rational.hh
@@ -74,7 +74,8 @@ public:
Rational (Rational const r) { copy (r);}
Rational operator = (Rational const r)
{
-copy (r); return *this;
+copy (r);
+return *this;
}
Rational operator *= (Rational);
@@ -89,11 +90,11 @@ public:
#include arithmetic-operator.hh
-IMPLEMENT_ARITHMETIC_OPERATOR (Rational, /);
+IMPLEMENT_ARITHMETIC_OPERATOR (Rational, / );
IMPLEMENT_ARITHMETIC_OPERATOR (Rational, +);
IMPLEMENT_ARITHMETIC_OPERATOR (Rational, *);
IMPLEMENT_ARITHMETIC_OPERATOR (Rational, -);
-IMPLEMENT_ARITHMETIC_OPERATOR (Rational, %);
+IMPLEMENT_ARITHMETIC_OPERATOR (Rational, % );
extra space before closing parenthesis
diff --git a/flower/rational.cc b/flower/rational.cc
index fbac400..2d57eeb 100644 (file)
--- a/flower/rational.cc
+++ b/flower/rational.cc
@@ -133,7 +131,6 @@ Rational::mod_rat (Rational div) const
return r;
}
-
/*
copy paste from scm_gcd (GUILE).
*/
@@ -162,7 +159,7 @@ gcd (I64 u, I64 v)
else
{
t = u;
-b3:
+b3:
goto label unindented
diff --git a/lily/audio-item.cc b/lily/audio-item.cc
index 5c9150f..4b3ee81 100644 (file)
--- a/lily/audio-item.cc
+++ b/lily/audio-item.cc
@@ -39,18 +39,12 @@ Audio_item::get_column () const
}
Audio_item::Audio_item ()
- : audio_column_ (0)
- , channel_ (0)
+ : audio_column_ (0), channel_ (0)
{
}
ctor init list move to single line
Cheers,
Neil
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Sun, Jul 03, 2011 at 02:25:12PM +0100, Neil Puttock wrote:
MatrixT, A (vsize rows, vsize columns, T const t)
- : data_(rows * columns, t)
+: data_ (rows *columns, t)
misinterpreted multiplication sign
heh, that one's funny!
--- a/flower/include/rational.hh
+++ b/flower/include/rational.hh
@@ -74,7 +74,8 @@ public:
Rational (Rational const r) { copy (r);}
Rational operator = (Rational const r)
{
-copy (r); return *this;
+copy (r);
+return *this;
}
That looks fine? astyle has a specific option to
--keep-one-line-statements, but I think those are evil?
-b3:
+b3:
goto label unindented
We could use --indent-labels for this
--- a/lily/audio-item.cc
+++ b/lily/audio-item.cc
@@ -39,18 +39,12 @@ Audio_item::get_column () const
}
Audio_item::Audio_item ()
- : audio_column_ (0)
- , channel_ (0)
+ : audio_column_ (0), channel_ (0)
{
}
ctor init list move to single line
I'm surprised astyle gets this wrong!
Cheers,
- Graham
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On 3 July 2011 14:39, Graham Percival gra...@percival-music.ca wrote:
On Sun, Jul 03, 2011 at 02:25:12PM +0100, Neil Puttock wrote:
MatrixT, A (vsize rows, vsize columns, T const t)
- : data_(rows * columns, t)
+ : data_ (rows *columns, t)
misinterpreted multiplication sign
heh, that one's funny!
--- a/flower/include/rational.hh
+++ b/flower/include/rational.hh
@@ -74,7 +74,8 @@ public:
Rational (Rational const r) { copy (r);}
Rational operator = (Rational const r)
{
- copy (r); return *this;
+ copy (r);
+ return *this;
}
That looks fine? astyle has a specific option to
--keep-one-line-statements, but I think those are evil?
Sorry, I didn't mean to copy this bit (only the next part for the label).
- b3:
+b3:
goto label unindented
We could use --indent-labels for this
--- a/lily/audio-item.cc
+++ b/lily/audio-item.cc
@@ -39,18 +39,12 @@ Audio_item::get_column () const
}
Audio_item::Audio_item ()
- : audio_column_ (0)
- , channel_ (0)
+ : audio_column_ (0), channel_ (0)
{
}
ctor init list move to single line
I'm surprised astyle gets this wrong!
On a slight tangent: we might want a coding policy decision for init
lists (I understand they're more efficient). So far, only Jan has
changed any files to use them (with the recent midi fixes).
Cheers,
Neil
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Sun, 03 Jul 2011 05:18:32 -0700, Carl Sorensen c_soren...@byu.edu wrote:
if (Score *newscore = unsmob_score (entry))
-*t = scm_cons (newscore-clone ()-unprotect (), SCM_EOL);
+* t = scm_cons (newscore-clone ()-unprotect (), SCM_EOL);
else if (Page_marker *marker = unsmob_page_marker (entry))
-*t = scm_cons (marker-clone ()-unprotect (), SCM_EOL);
+* t = scm_cons (marker-clone ()-unprotect (), SCM_EOL);
Astyle bug. She gets this wrong confused iff the condition contains an
assignment.
int
-Book::print_smob (SCM, SCM p, scm_print_state*)
+Book::print_smob (SCM, SCM p, scm_print_state *)
{
Astyle being consistent. GNU puts the * (and similarly, ) next to the name in
declarations, char *p, so we included --align-pointer=name in the astyle option
list.
On Sun, 03 Jul 2011 06:25:12 -0700, Neil Puttock n.putt...@gmail.com wrote:
A few fishy looking changes:
int lily_cookie_fprintf (void *file, char const *format, ...)
-__attribute__ ((format (printf, 2, 3)));
+ __attribute__ ((format (printf, 2, 3)));
Astyle bad. It would be better if she indents line-broken headers/statements,
when there are no operators to align on.
-templateclass T, class A=std::allocatorT
-class Matrix
+template class T, class A = std::allocatorT
+ class Matrix
Prefilter bad, I think. The original fixcc prefilter padded, then tried to un-pad,
these . I /thought/ I had removed all this.
MatrixT, A (vsize rows, vsize columns, T const t)
- : data_(rows * columns, t)
+: data_ (rows *columns, t)
Astyle bad. Thinks rows might be a typedef (but it cannot be, here).
-IMPLEMENT_ARITHMETIC_OPERATOR (Rational, /);
+IMPLEMENT_ARITHMETIC_OPERATOR (Rational, / );
IMPLEMENT_ARITHMETIC_OPERATOR (Rational, +);
IMPLEMENT_ARITHMETIC_OPERATOR (Rational, *);
IMPLEMENT_ARITHMETIC_OPERATOR (Rational, -);
-IMPLEMENT_ARITHMETIC_OPERATOR (Rational, %);
+IMPLEMENT_ARITHMETIC_OPERATOR (Rational, % );
Astyle inconsistent. Although macros do violate C syntax.
-b3:
+b3:
goto label unindented
G We could use --indent-labels for this
Probably best to let astyle un-indent goto labels, rather than follow the block
structure, because goto functions completely outside the block structure.
--- a/lily/audio-item.cc
+++ b/lily/audio-item.cc
@@ -39,18 +39,12 @@ Audio_item::get_column () const
}
Audio_item::Audio_item ()
- : audio_column_ (0)
- , channel_ (0)
+ : audio_column_ (0), channel_ (0)
{
}
ctor init list move to single line
G I'm surprised astyle gets this wrong!
Prefilter bad (so you are right to be surprised). It closes the linebreak
before a dangling comma, whereas it should swap ',' with '\n' -- or maybe just
leave these alone.
Looks like I should reduce the pre-filter further, but I'm tired of poring over
diffs and regexps (because I made several passes earlier) so I'll do it later
in the day. Thanks for setting up the branch, Graham.
Anything left in our pre-filter could be converted to patches to submit for Astyle, as
I've done already for foo ((bar *)fim).
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
On Sun, Jul 03, 2011 at 02:51:47PM +0100, Neil Puttock wrote: I'm surprised astyle gets this wrong! On a slight tangent: we might want a coding policy decision for init lists (I understand they're more efficient). So far, only Jan has changed any files to use them (with the recent midi fixes). Thanks, reminder pushed. I'll try to wrap it in with some other code discussion, but it probably won't be until August. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
I really, really, wish Astyle had a fully gnu-compliant mode out of the box. I went through the regexp's again and think this is good to go. I'll push a commit showing this run over the whole repository to http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=shortlog;h=refs/heads/dev/gperciva-astyle-keith http://codereview.appspot.com/4662074/diff/4031/lily/align-interface.cc File lily/align-interface.cc (right): http://codereview.appspot.com/4662074/diff/4031/lily/align-interface.cc#newcode375 lily/align-interface.cc:375: stacking-dir ); The prefilter in fixcc.py doesn't like the dangling ) http://codereview.appspot.com/4662074/diff/4031/lily/audio-item.cc File lily/audio-item.cc (right): http://codereview.appspot.com/4662074/diff/4031/lily/audio-item.cc#newcode43 lily/audio-item.cc:43: channel_ (0) The indenter in emacs un-indents the member initializers, whether we move the ',' or not, but the names remain aligned http://codereview.appspot.com/4662074/diff/4031/lily/beam-quanting.cc File lily/beam-quanting.cc (right): http://codereview.appspot.com/4662074/diff/4031/lily/beam-quanting.cc#newcode474 lily/beam-quanting.cc:474: std::priority_queue Beam_configuration *, std::vectorBeam_configuration *, If the template is broken into two lines, astyle does the angle braces wrongly http://codereview.appspot.com/4662074/diff/4031/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/4662074/diff/4031/lily/beam.cc#newcode70 lily/beam.cc:70: max_connect_ = 1000; // infinity prefilter in fixcc.py collapses spaces http://codereview.appspot.com/4662074/diff/4031/lily/context.cc File lily/context.cc (right): http://codereview.appspot.com/4662074/diff/4031/lily/context.cc#newcode283 lily/context.cc:283: add_listener (GET_LISTENER (new_context-create_context_from_event), astyle does not (yet) indent line-broken statements if there is no appropriate operator to align to http://codereview.appspot.com/4662074/diff/4031/lily/context.cc#newcode330 lily/context.cc:330: accepts); we can set the limit on how far astyle will indent to align with parentheses; I put 60 columns as the maximum http://codereview.appspot.com/4662074/diff/4031/lily/general-scheme.cc File lily/general-scheme.cc (right): http://codereview.appspot.com/4662074/diff/4031/lily/general-scheme.cc#newcode55 lily/general-scheme.cc:55: tail = SCM_CDRLOC (*tail); this is why I need the prefilter in fixcc.py http://codereview.appspot.com/4662074/diff/4031/lily/general-scheme.cc#newcode217 lily/general-scheme.cc:217: + default_value_string + '.); the prefilter in fixcc.py moves the + s http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py File scripts/auxiliar/fixcc.py (right): http://codereview.appspot.com/4662074/diff/4031/scripts/auxiliar/fixcc.py#newcode120 scripts/auxiliar/fixcc.py:120: \#[ \t]*define[ \t]+([^\n]*\\\n)*[^\n]*))''', I made a pattern to skip over defines, as it skips over comments, and leave them un-touched. The old method of un-doing the damage prefilter did to #defines, made me nervous. http://codereview.appspot.com/4662074/diff/4031/test.c File test.c (right): http://codereview.appspot.com/4662074/diff/4031/test.c#newcode1 test.c:1: # includefoo(bar(baz compare to Patch set 2 to see the effect of fixcc.py with astyle http://codereview.appspot.com/4662074/diff/4031/testfile_fixcc_emacs.c File testfile_fixcc_emacs.c (right): http://codereview.appspot.com/4662074/diff/4031/testfile_fixcc_emacs.c#newcode1 testfile_fixcc_emacs.c:1: /* testfile from fixcc.py, as *output* from fixcc.py using emacs */ The output of the old, more-picky fixcc+emacs is left unchanged by fixcc+astyle. (No diff set 2--3) http://codereview.appspot.com/4662074/diff/4031/testfile_in.c File testfile_in.c (right): http://codereview.appspot.com/4662074/diff/4031/testfile_in.c#newcode39 testfile_in.c:39: !OK OK These comments were messed-up on input, and they remain so on output. I just tested with the testfile; I didn't write it. http://codereview.appspot.com/4662074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Reviewers: ,
Message:
This script, applied to the .cc .icc and .hh files in flower/ and lily/,
gives a clean make check.
http://codereview.appspot.com/4662074/diff/7001/scripts/auxiliar/fixcc.py
File scripts/auxiliar/fixcc.py (right):
http://codereview.appspot.com/4662074/diff/7001/scripts/auxiliar/fixcc.py#newcode79
scripts/auxiliar/fixcc.py:79: # space after `operator'
Overloaded operators get a space? operator + () {
Description:
Adapt fixcc.py to use Astyle instead of emacs
In the prefilter, remove substitution rules
that duplicate the formatting that Astyle can do.
Remove a rule, removing space before a \-continuation,
which change emacs would have reversed.
Please review this at http://codereview.appspot.com/4662074/
Affected files:
M scripts/auxiliar/fixcc.py
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adapt fixcc.py to use Astyle instead of emacs (issue4662074)
Can you make it possible for us to see the diff caused by applying this script to the files you've mentioned? Thanks, Carl http://codereview.appspot.com/4662074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
