Add \a escape sequence #5864

wooster0 · 2018-03-25T17:36:46Z

The escape sequence that beeps/bells.

Right now when you do

puts "\x07"

then you can hear a beep/bell and it already works because 07 is the HEX value in ASCII of \a.
But I still think that we should add the escape sequence \a directly because it's really some kind of special feature. Thats pretty much the only thing you can hear in a terminal.

This escape sequence can be useful for example when you are making an timer or something like that. So you can notify the user while he's doing something else. And it can be also helpful at debugging something without a display. Or generally debugging.

The escape sequence that makes a beep.

oprypin · 2018-03-25T17:44:35Z

I think when the original developer was deciding for the first time what escape sequences to include, they made an educated decision on which ones are important. I don't think it's a matter of forgetting to include it or anything like that. More is not always better. Ruby having it is not a good argument.

oprypin · 2018-03-25T17:50:36Z

Now that I look at it more carefully, this does seem like an unusual omission, because Crystal has all other escape sequences from these two

oprypin · 2018-03-25T17:56:31Z

spec/compiler/lexer/lexer_spec.cr

@@ -230,6 +230,7 @@ describe "Lexer" do
  it_lexes_number :i8, ["0i8", "0"]

  it_lexes_char "'a'", 'a'
+  it_lexes_char "'\\a'", '\a'


I think to keep release compatibility you would have to write 7.chr not '\a'

Sija · 2018-03-25T18:03:14Z

src/compiler/crystal/syntax/lexer.cr

@@ -493,6 +493,8 @@ module Crystal
            case char
            when '\\'
              case char = next_char
+              when 'a'
+                io << "\x07"


I'd keep "\u{7}" format.

Yeah basically you have to do whatever the \b addition does. New source code must still compile with the old compiler.

Sija · 2018-03-25T18:03:25Z

src/compiler/crystal/syntax/lexer.cr

@@ -1789,6 +1793,8 @@ module Crystal
            end
          else
            case char = next_char
+            when 'a'
+              string_token_escape_value "\x07"


wooster0 · 2018-03-26T06:15:37Z

Ok now I did whatever the \b addition does. In every file. I searched through every single file for \b.

But CircleCI says:

Syntax error in src/string.cr:3942: invalid char escape sequence



      when '\a' then io << "\\a"

           ^

oprypin · 2018-03-26T06:29:01Z

Can't use \a anywhere, except when it's \\a

oprypin · 2018-03-26T06:38:16Z

4f462ba#diff-87ebb896824e7dc3bd8d5b29293bcf00R778

Sija · 2018-04-05T15:53:59Z

@RX14 \b is unprintable as well, I see no reason to skip this one, while having all others included. This should be added only if for completeness sake.

j8r · 2018-04-05T16:14:19Z

Agree, there are also some languages that support it like Ruby and Python, and others don't like Rust ~~and Go~~ AFAIK.

straight-shoota · 2018-04-05T16:18:36Z

Go supports \a, Rust does not.

RX14 · 2018-04-06T09:50:53Z

@Sija we should remove \b.

RX14 · 2018-04-06T09:56:13Z

It seems rust supports only \u{abcde} (only with curly braces!), \xAB, and then \n, \r, \t, and \0. And obviously \\. We should duplicate that.

Sija · 2018-04-06T09:57:15Z

@RX14 or add \a...

RX14 · 2018-04-06T10:37:15Z

\a is platform-specific and obsolete, so is \b. Should we add \f? \v? \e too? No, let's support only the common subset and everything else can be done just fine with \u{}.

straight-shoota · 2018-04-06T18:00:27Z

\b\f\v\e are actually recognized (see https://crystal-lang.org/docs/syntax_and_semantics/literals/string.html ). I think we all agree we should either remove all of them or add \a. I don't have a strong position on this. They are probably mostly useless - but on the other hand it doesn't really hurt to have them and in rare instances it might help interoprability.

@RX14 How is \a platform-specific? It's just an alternative name to express codepoint 7.

RX14 · 2018-04-06T20:08:13Z

I absolutely agree we should add all of them or remove all of them except r, n, t. I just think we should remove complexity, and arcane, typically unused escape sequences.

straight-shoota · 2018-04-06T20:36:23Z

I'd say \e should also stay. It is even used in Colorize and Benchmark::IPS

Sija · 2018-04-06T20:40:54Z

@RX14 What's the point of removing well-known functionalities (even if they indeed have arcane heritage), just for removal sake? Wouldn't be better to stay compatible with other languages and be modern in ways that are meaningful?

asterite · 2018-04-06T22:38:48Z

I didn't include "\a" because I don't find it useful. I wouldn't mind having it in the language, though.

RX14 · 2018-04-06T22:51:05Z

I don't care enough to bikeshed over it. If this many people feel it should be in the language, then just add it and I won't use it. Either way this PR (clearly done from the github web UI) wasn't very good.

asterite · 2018-04-28T13:40:44Z

(if someone approves this and wants to merge this, please squash :-)

Sija · 2018-04-28T14:28:43Z

Also, json additions need to be removed, since \a is invalid character in JSON string.

straight-shoota · 2018-04-28T19:25:37Z

spec/std/string_spec.cr

@@ -1444,6 +1444,7 @@ describe "String" do
    "a".dump.should eq %("a")
    "\\".dump.should eq %("\\\\")
    "\"".dump.should eq %("\\\"")
+    # TODO: "\a".dump.should eq %("\\a")


Maybe add a note uncomment after 0.24.2?

I think this is enough. Right after 0.25.0 gets released, I will finish this with another PR and there I will just look here to see what I need to do.

straight-shoota · 2018-04-28T19:27:28Z

src/compiler/crystal/syntax/lexer.cr

@@ -493,6 +493,8 @@ module Crystal
            case char
            when '\\'
              case char = next_char
+              when 'a'
+                io << "\u{7}"


Is there a reason these were not touched by #5882 @Sija?

Nope, just an omission.

Should I make a PR to also make these strings chars?

I'd suggest to merge this one first.

wooster0 added 2 commits March 25, 2018 19:02

Add \a escape sequence

3148078

The escape sequence that makes a beep.

Update lexer_spec.cr

2b20c37

oprypin reviewed Mar 25, 2018

View reviewed changes

7.chr

b22d159

Sija reviewed Mar 25, 2018

View reviewed changes

wooster0 added 9 commits March 25, 2018 20:07

\u{7}

fdbe891

Update string.cr

e0922ea

Update char_spec.cr

46e8fa6

Update parser_spec.cr

556f4a1

Update string_spec.cr

1fd04ab

Update builder.cr

499f326

Update lexer.cr

3eeba60

Update serialization_spec.cr

07933cb

Update lexer_spec.cr

13d0ae3

wooster0 added 10 commits March 26, 2018 09:13

Update lexer.cr

771cab1

Update string.cr

ef3ba45

Update char_spec.cr

e0f1cef

Update string_spec.cr

a1ab033

Update builder.cr

3d84815

Update lexer.cr

8020d77

Update string_spec.cr

17ad4b3

Update lexer_spec.cr

8ee81da

Update serialization_spec.cr

81435b6

Update lexer_string_spec.cr

b709308

wooster0 closed this Apr 5, 2018

wooster0 reopened this Apr 7, 2018

asterite approved these changes Apr 28, 2018

View reviewed changes

wooster0 added 4 commits April 28, 2018 16:33

Remove \a in json lexer.cr

a37c81f

Remove \a in json lexer_spec.cr

8e51d58

Remove \a in json serialization_spec.cr

9f5073b

Remove \a in json builder.cr

672bea2

straight-shoota reviewed Apr 28, 2018

View reviewed changes

sdogruyol approved these changes May 11, 2018

View reviewed changes

sdogruyol merged commit 1ecc65b into crystal-lang:master May 11, 2018

sdogruyol added topic:compiler kind:feature labels May 11, 2018

sdogruyol added this to the Next milestone May 11, 2018

chris-huxtable pushed a commit to chris-huxtable/crystal that referenced this pull request Jun 6, 2018

Add \a escape sequence (crystal-lang#5864)

ff128c8

bcardiff mentioned this pull request Jun 21, 2018

Finish the \a escape sequence #6221

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add \a escape sequence #5864

Add \a escape sequence #5864

wooster0 commented Mar 25, 2018 •

edited

Loading

oprypin commented Mar 25, 2018

oprypin commented Mar 25, 2018

oprypin Mar 25, 2018

Sija Mar 25, 2018

oprypin Mar 25, 2018

Sija Mar 25, 2018

wooster0 commented Mar 26, 2018 •

edited

Loading

oprypin commented Mar 26, 2018

oprypin commented Mar 26, 2018 •

edited

Loading

Sija commented Apr 5, 2018

j8r commented Apr 5, 2018 •

edited

Loading

straight-shoota commented Apr 5, 2018

RX14 commented Apr 6, 2018

RX14 commented Apr 6, 2018 •

edited

Loading

Sija commented Apr 6, 2018

RX14 commented Apr 6, 2018 •

edited

Loading

straight-shoota commented Apr 6, 2018

RX14 commented Apr 6, 2018

straight-shoota commented Apr 6, 2018

Sija commented Apr 6, 2018 •

edited

Loading

asterite commented Apr 6, 2018

RX14 commented Apr 6, 2018 •

edited

Loading

asterite commented Apr 28, 2018

Sija commented Apr 28, 2018

straight-shoota Apr 28, 2018

wooster0 Apr 28, 2018 •

edited

Loading

straight-shoota Apr 28, 2018

Sija Apr 28, 2018

wooster0 Apr 30, 2018

Sija Apr 30, 2018

Add \a escape sequence #5864

Add \a escape sequence #5864

Conversation

wooster0 commented Mar 25, 2018 • edited Loading

oprypin commented Mar 25, 2018

oprypin commented Mar 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wooster0 commented Mar 26, 2018 • edited Loading

oprypin commented Mar 26, 2018

oprypin commented Mar 26, 2018 • edited Loading

Sija commented Apr 5, 2018

j8r commented Apr 5, 2018 • edited Loading

straight-shoota commented Apr 5, 2018

RX14 commented Apr 6, 2018

RX14 commented Apr 6, 2018 • edited Loading

Sija commented Apr 6, 2018

RX14 commented Apr 6, 2018 • edited Loading

straight-shoota commented Apr 6, 2018

RX14 commented Apr 6, 2018

straight-shoota commented Apr 6, 2018

Sija commented Apr 6, 2018 • edited Loading

asterite commented Apr 6, 2018

RX14 commented Apr 6, 2018 • edited Loading

asterite commented Apr 28, 2018

Sija commented Apr 28, 2018

Choose a reason for hiding this comment

wooster0 Apr 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wooster0 commented Mar 25, 2018 •

edited

Loading

wooster0 commented Mar 26, 2018 •

edited

Loading

oprypin commented Mar 26, 2018 •

edited

Loading

j8r commented Apr 5, 2018 •

edited

Loading

RX14 commented Apr 6, 2018 •

edited

Loading

RX14 commented Apr 6, 2018 •

edited

Loading

Sija commented Apr 6, 2018 •

edited

Loading

RX14 commented Apr 6, 2018 •

edited

Loading

wooster0 Apr 28, 2018 •

edited

Loading