stata_kernel icon indicating copy to clipboard operation
stata_kernel copied to clipboard

Else block always runs

Open kylebarron opened this issue 7 years ago • 24 comments

@mcaceresb This is because we run other commands after the if clause gets sent. Not really sure the best way to fix this. In Jupyter Notebook/Atom you can just be careful to only send the entire if/else block. In Jupyter console, you can fix this by wrapping the entire block in a pair of braces.

image image

Console workaround: image

kylebarron avatar Sep 07 '18 19:09 kylebarron

If there is more than one line of code, simply require two blank lines instead of one for the chunk to be considered complete.

mcaceresb avatar Sep 07 '18 19:09 mcaceresb

(i.e. two <CR> from the user)

mcaceresb avatar Sep 07 '18 19:09 mcaceresb

Brilliant!

kylebarron avatar Sep 07 '18 19:09 kylebarron

This is helpful for the console, but the behavior is still suboptimal in Hydrogen, since it doesn't use is_complete messages at all.. peek 2018-09-07 15-27

kylebarron avatar Sep 07 '18 19:09 kylebarron

The only way to get around this in hydrogen right now is to manually select the if/else block. For now I should at least put this in the limitations page in the docs.

peek 2018-09-07 15-28

kylebarron avatar Sep 07 '18 19:09 kylebarron

Wait, how does it do it for python, then?

mcaceresb avatar Sep 07 '18 19:09 mcaceresb

It doesn't. Since Hydrogen is 100% language-agnostic, it uses only general syntax elements like indentation and the foldEndMarker designated by the language's syntax highlighting package.

Recently a separate python-specific add-on package was developed to expand code lines. See https://github.com/nikitakit/hydrogen-python/pull/10 for the difficulty of correctly implementing this feature.

This is something Hydrogen struggles with in general because of its language-independentness... Here's the same issue in Javascript. peek 2018-09-07 15-47

I think it's something that could be improved in Hydrogen... If you had a suggestion I might mention it to the other maintainers and try to create a PR for it.

kylebarron avatar Sep 07 '18 19:09 kylebarron

and the foldEndMarker designated by the language's syntax highlighting package.

I thought you were in charge of that, or is it too much of a pain to implement? You can nest if/else in an ifelse fold if if is followed by else in the next line?

mcaceresb avatar Sep 07 '18 20:09 mcaceresb

I am. And I edited it recently to include the ending } in Hydrogen.

But it's just a plain regex setting (that a core Atom package then implements) and Atom can't do regexes across lines anyways. So I don't think it's possible to say "} ends folds except when else { or else if { appears on the following line"

kylebarron avatar Sep 07 '18 20:09 kylebarron

In Javascript, you could put }(?!\s* else) as the regex and plausibly get the whole if/else clause. But not sure how to do it when they're always on separate lines.

kylebarron avatar Sep 07 '18 20:09 kylebarron

Multi-line regex?

mcaceresb avatar Sep 07 '18 20:09 mcaceresb

Atom can't do regexes across lines anyways

At least not for syntax highlighting. I doubt it allows it here, though I could try it out.

kylebarron avatar Sep 07 '18 20:09 kylebarron

Wait, this is odd. Python has multi-line strings. Doesn't it highlight them?

Conceptually, if should start an if-else environment ended by a blank line after } or an else block, right? I reckon that's not how it's written?

mcaceresb avatar Sep 07 '18 20:09 mcaceresb

Wait, this is odd. Python has multi-line strings. Doesn't it highlight them?

Yes, but the token that it looks for in the regular expression never spans a newline. I.e. the """ will be on one line and another, and Atom can create regions between the lines, but the token can never span lines.

That's why it's impossible, for example, to highlight Setext headers in Markdown https://github.com/burodepeper/language-markdown/issues/192

kylebarron avatar Sep 07 '18 20:09 kylebarron

Actually, it is ended by an else block or a line containing anything that doesn't start with else

mcaceresb avatar Sep 07 '18 20:09 mcaceresb

But that is not the same, tho, right? Because you can look for environment delimiters...at least that is what I'm assuming.

mcaceresb avatar Sep 07 '18 20:09 mcaceresb

The markdown example has no delimiter to start the header, just to end it.

mcaceresb avatar Sep 07 '18 20:09 mcaceresb

With the syntax highlighting yes. But not the fold end marker to my knowledge. I haven't tried but I'd be surprised.

kylebarron avatar Sep 07 '18 21:09 kylebarron

What's a simple regex to try?

}(?!\s*\n\s*else)

kylebarron avatar Sep 07 '18 21:09 kylebarron

Something like that, yeah. Remember that you can have if w/o brackets and else with brackets after afaik

mcaceresb avatar Sep 07 '18 21:09 mcaceresb

It actually works when not using brackets: image

I suppose the main reason is that when there are brackets I run the code with include

kylebarron avatar Sep 07 '18 21:09 kylebarron

So include must break Stata's record of if/else somehow

kylebarron avatar Sep 07 '18 21:09 kylebarron

The only case in which it doesn't work is when if uses brackets.

Why did we end up using include anyways?

peek 2018-09-07 17-10

kylebarron avatar Sep 07 '18 21:09 kylebarron

Oh right we use include so that commands will stop running after one creates an error. Which is good...

kylebarron avatar Sep 07 '18 21:09 kylebarron