GeezerVison

The initial condition was based on tok->multi_line_start but then revisiting PEP701 I noticed that the example:

>>> f'''A complex trick: {
... bag['bag']  # recursive bags!
... }'''

contained the opening curly bracket on the same level, so it was failing, but all the other cases were not.

My goal was to skip that error condition, but I couldn't make a difference from the following cases:

f"something {#

and

f"""something {# some lines ahead
...

so I decided not to accept comments on the first line.

Ideas are welcome :)

@lysnikolaou @isidentical @mgmacias95 thoughts?

Well, I'm not sure I like the idea of disallowing comments in the first line. One of the main points of 701 was that explaining f-string features would be much easier and this goes in the opposite direction. "You can add comments in f-string expressions" is much easier to remember than "You can add comments in f-string expressions, but not the first line". And this already comes on top of only allowing them in multi-line f-strings.

This is not a very strong opinion and I'd obviously be okay to go with it, if working around it is very difficult and there's consensus that this is okay.

Another option we could have, is start iterating on the rest of the line characters, and if I find a '}' I know we are on the same line and it's not valid, and if we find a '\n' we know it's a multi-line expression.

Meaning:

>>> f"nope {#}"
  File "<stdin>", line 1
    f"nope {#}"
             ^
SyntaxError: f-string expression part cannot include '#'
>>> f""" yep { # something
... 1 + 2 # math!
... }"""
' yep 3'
>>> 

Can update the PR, but the code will look like this:

          if (INSIDE_FSTRING(tok) && (tok->lineno == 1)) {                                               
             while (c != '\n') {                                                                        
                 c = tok_nextc(tok);                                                                    
                 if (c == '}')                                                                          
                     return MAKE_TOKEN(syntaxerror(tok, "f-string expression part cannot include '#'"));
             }                                                                                          
          }

Do you think something like this would make more sense @lysnikolaou ?

If people gets creative I we will have problems as well with cases like:

>>> f"meh {# I really like the } character
  File "<stdin>", line 1
    f"meh {# I really like the } character
                               ^
SyntaxError: f-string expression part cannot include '#'

edit: if tok>lineno == 1 doesn't look too good, we could also condition it wrt the quote size, so we could have current_tok->f_string_quote_size == 1 instead. After all, we could detect if we are on a possible multi-line string.
This might open another strange-looking code:

>>> f"""{#}"""
... 42}"""
'42'

but maybe it's better?

edit: if tok>lineno == 1 doesn't look too good, we could also condition it wrt the quote size, so we could have current_tok->f_string_quote_size == 1 instead. After all, we could detect if we are on a possible multi-line string.

This is the way to go in my opinion. The snippet you showed, while a bit tricky to parse for a human at first, should be okay, since this is also the case outside of f-strings:

a_set = (#{
    {
        1, 2, 3, 4#}
    }
)

This is the way to go in my opinion.

I agree with this 👍

I think that the simpler the rules the easier to explain, specially since most syntax highlighters will do the right thing and show the comment in a darker shade

Thanks for your feedback 🎉
Let me know if something else then is required for this before it's merged.

Hey @cmaureir, any progress on this? Will you get some time to address the comments so that we can merge this?

Hey @cmaureir, any progress on this? Will you get some time to address the comments so that we can merge this?

Hey, I got lost in the corner-cases, and I got distracted afterwards.
I tried to think of a mechanism that can enable things like this:
f"something {1 # one} and {2 # two}"
but some solutions are leaving out the cases that Pablo was highlighting, like:

>>> f"{1 # look some weird escaped }}
+
1
    }"

So I'd need some input if
(1) the moment I found a comment, then we don't care what's there and we know it's a comment until '\n'
e.g.:

f"something {1 # this is a comment } { no matter what is around on the rest of the line...
... }"

or
(2) if we start a comment, we iterate the characters until we find the first }

f"something {1 # this is a comment } <- the expr and comment ended there, the rest is just a str"

With (1), we simple remove the error from the tokenizer, and we would expect behavior like this (from Pablo's comments):

>>> a = 1
>>> f"{a # {1,2,3,4}   }"
... }"
'1'

>>> f"{a # {1,2,3,4}
... }"
'1'
>>> f"{1 # look some weird escaped }}
... +
... 1
... }"
'2'

What do you think @lysnikolaou would what be enough?

Yeah, let's go with (1).

For that behavior, then it should be simpler, and only removing the error condition will be enough. If more test cases are required, please let me know.

Btw, let's add some more tests, at least covering comments in single-quoted f-string as well, in a different PR.

Btw, let's add some more tests, at least covering comments in single-quoted f-string as well, in a different PR.

Understood. Thanks for your time on this :)