Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: implement #= ... =# multiline comments #6128

Merged
merged 1 commit into from Mar 12, 2014

Conversation

stevengj
Copy link
Member

This fixes #69, implementing #= ... =# multiline comments.

(Several people had suggested #= ... =# instead, but #{ ... }# has the advantage of exploiting automatic editor brace matching out of the box. Pascal forever!) Changed to #= .. =# to make accidental multiline comments less likely (see below).

It doesn't interact nicely with the REPL yet, because the REPL doesn't realize that #{ ... means that it should wait for more input. @loladiro, where is the end-of-input heuristic implemented in the REPL?

I haven't documented it yet, for the simple reason that comment syntax is not currently documented anywhere(!) in the Julia manual as far as I can tell, and I couldn't figure out what section it should go into.

@jakebolewski
Copy link
Member

Thanks so much for this. Does this have a hope of getting into 0.3?

@StefanKarpinski
Copy link
Sponsor Member

I shamelessly ripped this off but changed it to support #= ... =# syntax: #6129. [ducks]

@stevengj
Copy link
Member Author

  • Editor brace matching is a huge usability advantage, not to be discarded lightly.
  • I'm not convinced that one version is aesthetically so much nicer than the other.
  • Even if #= ... =# looks a bit nicer, I'm not sure that looks are so important here. Multiline comments are mainly useful for quick debugging hacks to comment out large blocks of code. We should really encourage people to continue to use a uniform style of # .... for permanent multiline documentation. (In which case ugly multiline comments are a feature, not a bug.)

(Copy-and-pasting my comments because that's what redundant PRs are for.)

@thomasmcoffee
Copy link

Without having tried it, I'm wondering what happens here:

function foo()
    {1, 0}  # option 1
    #{0, 1} # option 2
end

... many lines of code ...

function bar()
    x = 1 # assign x
    y = 2 # assign y
    {x, y}# result
end

In the worst case, if a variable named "result" were assigned somewhere, might not this compile but be completely wrong?

This example is even more troubling than the one I posed at #69 (comment)

@StefanKarpinski
Copy link
Sponsor Member

Good point. The advantage of #= ... =# is that = is not a valid way to begin or end an expression.

@stevengj
Copy link
Member Author

@StefanKarpinski, it's not true that you cannot end a line with = (the parser will just look to the next line for the right-hand-side), and you can certainly begin a comment with =.

This could theoretically happen with any #?, delimiter, even =. e.g.:

function foo(x)
   x+1
   #== 1 + x
end

function bar(x)
  x =# abs(x) # commented out old definition
      abs(x)+1
  return x
end

The basic issue here is that any two-character multiline comment delimiter #? ... ?#, for any ? that is used in Julia (i.e. all practical punctuation characters), will probably have this kind of theoretical vulnerability to accidental collisions. I'm not sure there is a way around it.

@StefanKarpinski
Copy link
Sponsor Member

It can, but editors tend to automatically prefix a bunch of lines with the comment character for you and people often put comments at the ends of lines. Now this can be mitigated by inserting spaces no matter what multiline combination we pick, but = is one of the least likely punctuation marks to be at the beginning or end of a line.

@stevengj
Copy link
Member Author

@StefanKarpinski, fair enough. I've amended the patch to use #= ... =#. It's a shame to lose brace matching, but minimizing collisions seems more important.

@StefanKarpinski
Copy link
Sponsor Member

Here are starting frequencies of leading characters in base:

$ perl -nle 'print $1 if /^\s*(\W)/' base/*.jl | grep '\S' | sort | uniq -c | sort -nr
   2738 #
    891 @
    486 (
    131 -
    106 :
    100 <
     95 .
     91 $
     54 "
     48 +
     46 >
     45 *
     37 !
     34 =
     32 ^
     28 /
     23 '
     21 |
     19 &
     13 ~
     12 [
     11 )
      6 \
      5 {
      4 ]
      2 }
      2 %

and ending frequences:

$ perl -nle 'print $1 if /(\W)\s*$/' base/*.jl | grep '\S' | sort | uniq -c | sort -nr
  14618 )
   2586 ,
    943 ]
    347 #
    267 =
    242 }
    241 .
    214 :
    171 "
     61 |
     57 '
     49 ;
     39 ?
     29 -
     26 !
     21 &
     17 (
     11 /
      8 *
      7 +
      6 [
      6 >
      4 `
      2 {
      2 %
      1 ^
      1 \

@stevengj
Copy link
Member Author

Now, back to the technical questions:

  • Where should this be documented?
  • Where is the REPL's "end-of-input" heuristic implemented, since that needs modification?

@stevengj
Copy link
Member Author

@StefanKarpinski, then maybe we should use #% ... %# to minimize collisions (and as a nod to Matlab % comments)?

Or go back to #{ .. }# and regain brace-matching, since brace actually seems no worse than = in this sense?

However, those statistics may be misleading. The main reason that I generally put = on the end of a line is in function definitions foo(x) = bar when the line is too long already.... in such cases, I can't see myself putting a comment after the =.

@ivarne
Copy link
Sponsor Member

ivarne commented Mar 12, 2014

Those statistics are misleading because Base is written to be pretty and readable. The question is what is convenient for debugging and quick modifications. I might want to format different when something fails and I want to comment out some part of the code.

@stevengj
Copy link
Member Author

@ivarne, so therefore what delimiter are you arguing for, or what statistic should we look at?

@StefanKarpinski
Copy link
Sponsor Member

I agree that the statistics need to be interpreted a bit – and trailing = are unlikely to be followed immediately by a comment. I think the leading characters are much more important because of the way editors do block comments. Putting a trailing comment right after code with no space is a bit contrived, imo. Almost all of the lines that start with = in base are definitions of methods for == – that's very unlikely to be as common in normal code. All of the trailing = are from split short form function definitions. The instances of { and } come from untyped array/dict literals and comprehensions. Those are naturally rare in base since we tend to want to type things tightly, but in user code, it's quite possible that { and } will be much more common. I'm not super into #% ... %# but the statistics do suggest it. On the other hand % is an operator and therefore a perfectly valid expression, while = is not.

@ivarne
Copy link
Sponsor Member

ivarne commented Mar 12, 2014

@stevengj The best statistic (in my opinion) would be to make a poll that present the different solutions with pros and possible conflicts/ambiguities. This is about user convenience and pretty looking code, not technical reasoning. Stefans last comment is spot on by the way.

@mweastwood
Copy link
Contributor

How about something like:

##
    I am a multi-
    line comment
##

Edit: I missed the discussion in #69 (ignore this)

@StefanKarpinski
Copy link
Sponsor Member

This is fine. Let's just go with #= ... =#.

StefanKarpinski added a commit that referenced this pull request Mar 12, 2014
WIP: implement #= ... =# multiline comments
@StefanKarpinski StefanKarpinski merged commit 49ea168 into JuliaLang:master Mar 12, 2014
@JeffBezanson
Copy link
Sponsor Member

Aww but issue #69 doesn't even have 100 comments yet.

@StefanKarpinski
Copy link
Sponsor Member

I'll be thrilled to never hear a word about multiline comments again.

@JeffBezanson
Copy link
Sponsor Member

Not likely :)

@tknopp
Copy link
Contributor

tknopp commented Mar 12, 2014

Time to spam base with pull requests containing multiline comments ;-)

jiahao added a commit that referenced this pull request Mar 12, 2014
@thomasmcoffee
Copy link

If it's really intended as an emergencies-only feature, there's always #==== ... ====#. [ducks]

@waldyrious
Copy link
Contributor

Interestingly enough, re-reading the whole thread of #69 one can clearly notice that #= ... =# was the most preferred option. Just to make sure this doesn't get lost now that the issue is closed, it would be nice to make sure that this supports the self-closing form #=# that several people agreed was desirable (and noone opposed, FWIW).

@StefanKarpinski
Copy link
Sponsor Member

Remind me what the point of #=# is...

@carlobaldassi
Copy link
Member

It seems nesting is not allowed. I think it would be nice if it was.

@thomasmcoffee
Copy link

I think the idea was to facilitate toggling the commenting of a section (changing #= => #=# and =# => #=#) without re-finding the boundary points.

You can do a similar thing in general without any special rules, with one more keystroke, changing #= => ##= and =# => #==#.

@stevengj
Copy link
Member Author

Fixed nesting and threw in #=# for good measure.

@carlobaldassi
Copy link
Member

Thanks!

@vtjnash
Copy link
Sponsor Member

vtjnash commented Mar 13, 2014

Where's the tests? Also, somewhere you should mention that these are nesting.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Mar 13, 2014

The REPL heuristics exist in several places. They are triggered when parsing returns an expression of the following form: Expr(:incomplete, "description")

(edit: fixed the form of the expression)

@stevengj
Copy link
Member Author

Thanks @vtjnash, will look into it.

(You know, I marked this pull request as a "WIP" for a reason.)

stevengj added a commit that referenced this pull request Mar 13, 2014
@stevengj stevengj mentioned this pull request Apr 7, 2014
@stevengj stevengj deleted the multiline-comments branch February 3, 2016 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code decision A decision on this change is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multiline comments