jung6717 / arduino

Automatically exported from code.google.com/p/arduino
0 stars 0 forks source link

Auto Format splits long comments onto a second line, loses a character and doesn't comment the second line #255

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Enter the following comment "  // this is a very long comment which
wraps around and stops being a comment at the end of it when the autoformat
command is used, which is a bug"
2. Hit Auto Format
3. It produces "    // this is a very long comment which wraps around and
stops being a comment at the end of it when the autoformat command is used, whi
  h is a bug"

N.B. it's dropped the "c" from which and not commented the "h is a bug"
line, so the code no longer compiles

What is the expected output? What do you see instead?

To either leave it on one line or split, ideally on a space/word boundary
and comment the second line, e.g. 

"  // this is a very long comment which wraps around and stops being a
comment at the end of it when the autoformat command is used,
  //which is a bug"

What version of the Arduino software are you using? On what operating
system?  Which Arduino board are you using?

IDE 0018 on Win XP, Java 1.6.0_13. Duemilanove board, not that it should
make a difference.

Original issue reported on code.google.com by peterjne...@gmail.com on 11 May 2010 at 1:37

GoogleCodeExporter commented 9 years ago
Still there on 1.0 on Win7.

Original comment by lars.j.n...@gmail.com on 8 Mar 2012 at 3:53

GoogleCodeExporter commented 9 years ago
I'm not surprised. The code that does the formatting is really confusing, and 
logically a total mess.

There are at least two bugs here:

1. The cpp_comment() method is throwing away a character when it exits the loop 
looking for a line end or the hard-coded max line length.
2. The main loop that reformats the code doesn't actually ever prepend the new 
line with "//"

I've got the start of a diff, but I still need to figure out how to break the 
comments on a word boundary -- without breaking anything else. I'm also really 
tempted to hard-code the line length to 72, because 132-column editors went out 
of style with polyester flare trousers.

But, honestly, the formatter needs to be completely rewritten.

Original comment by john.ve...@gmail.com on 10 May 2012 at 2:08

GoogleCodeExporter commented 9 years ago
I am not proud of this patch, but it does the trick. I tried to localize the 
changes to a single method.

It will break an arbitrary C++ style comment line into as many ~72-char lines 
as possible, with newlines at word breaks. I have tried my best not to 
introduce spurious newlines, which is a real problem in this code.

This code doesn't grok the notion of system specific line-ends, but neither 
does any of the autoformat stuff. It also imposes a 72-char line limit instead 
of the 132-char line limit. Both of these are completely arbitrary, and ideally 
would be driven off the window size, or a preference.

Consider this bugwards-compatible maintenance hacking. 

Original comment by john.ve...@gmail.com on 16 May 2012 at 2:28

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the patch.  BTW, have you looked at the Processing auto-format code 
lately?  I'm guessing they might have fixed this problem (and possibly improved 
the code in other ways too).  We could port over their improvements, allowing 
us to stay in sync and take advantage of future improvements that they make.

Original comment by dmel...@gmail.com on 23 May 2012 at 2:57

GoogleCodeExporter commented 9 years ago
I'll grab a copy of Processing and see what has been done over there.

Original comment by john.ve...@gmail.com on 26 May 2012 at 5:06

GoogleCodeExporter commented 9 years ago
Processing doesn't have much of the offending code, which is weird. It looks 
like what is in Arduino was never merged back into Processing, or Processing 
tore it out (I'd have to look at the history, which I admit I have not done.)

The upshot is that Processing doesn't bother auto-formatting long C++ comments 
at all. You can paste a gigantic line and it sees no need to reformat. I'd 
consider this a bug, but they might have decided this was acceptable.

Original comment by john.ve...@gmail.com on 6 Jun 2012 at 1:56

GoogleCodeExporter commented 9 years ago
Interesting.  Do you think it make sense to go with your patch anyway?  Do you 
have any example sketches that, when auto-formated, illustrate the various 
cases of its behavior?

Original comment by dmel...@gmail.com on 6 Jun 2012 at 2:00

GoogleCodeExporter commented 9 years ago
Ok, what is in Arduino looks similar to the same file in Processing, circa 
r6536 from 2010 (or later). Ideally we would just want to pull a lot of this 
in, but there have been a lot of changes made since then, including a lot of 
refactoring to allow for unit tests and so on. So, there is a fair amount of 
refactoring to do. I don't have a good feel for the full scope of the work, and 
I'm really a novice where it comes to negotiating between subversion and git. 
My merge-fu will be weak.

And, like I said, the tiprev Processing doesn't actually solve the problem 
reported here -- it just reports "No changes necessary for AutoFormat") 

I've attached a simple sketch that exercises my change and to minor edge 
conditions.

Original comment by john.ve...@gmail.com on 30 Jun 2012 at 1:27

Attachments:

GoogleCodeExporter commented 9 years ago
I should add that my patch, by complete oversight, will not break lines that 
have no spaces in them, at least after some critical point in the line. I'm 
going to consider this a feature that ugly commenting that people like to do 
with long strings of dashes or asterisks will be preserved.

That's my story, and I'm sticking to it.

Original comment by john.ve...@gmail.com on 30 Jun 2012 at 1:29

GoogleCodeExporter commented 9 years ago
Thanks again for looking into this.  I tried out your patch and had a few 
concerns, so for now, I just committed a change to stop auto-format from 
breaking C++ style comments at all:

https://github.com/arduino/Arduino/commit/6030f9670b11389a054470a38f39ffcf8c2c6d
85

In particular, breaking (wrapping) the comments can create awkward to read text 
in a few situations.  One is where the comment starts at the end of a line of 
code.  Because the second (and subsequent) lines of the comments start at the 
beginning of the line, it's not clear that they're continuations of the comment 
on the previous line (which started after the code).  Also, if the sketch 
author has already wrapped the comment, but to a longer line (e.g. 80 
characters), you can end up with awkward text which is, say, 70 characters 
followed by 10, followed by 70, another 10, etc.  These kinds of cases made it 
seem like the patch's behavior was leading to text that was harder to read than 
if it had just been left alone.  Some sort of more sophisticated comment 
wrapping algorithm (which handled these cases properly) would probably even 
better than not wrapping at all, but it seems like a complicated thing to get 
right.  

I'm marking this as fixed for now (since the bug no longer occurs) but it's 
probably possible to improve on the behavior in other ways.  

Original comment by dmel...@gmail.com on 15 Jul 2012 at 1:01