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

shopt parse_equals should only be on in config mode #937

Closed
andychu opened this issue May 14, 2021 · 5 comments
Closed

shopt parse_equals should only be on in config mode #937

andychu opened this issue May 14, 2021 · 5 comments

Comments

@andychu
Copy link
Contributor

andychu commented May 14, 2021

  • Oil mode should use const.
  • config mode can be "cows" or "hay"
@andychu
Copy link
Contributor Author

andychu commented Apr 2, 2022

A user brought this same issue up here. So I agree we need to address this

https://oilshell.zulipchat.com/#narrow/stream/121540-oil-discuss/topic/Require.20the.20.60const.60.20keyword

@andychu
Copy link
Contributor Author

andychu commented Apr 3, 2022

Current behavior

bin/osh

a=42
const a = 42
a = 42  # not what you think: runs the command a

bin/oil

a=42  # disallowed for compatibility
const a = 42 

a = 42  # equivalent to `const a = 42` right now

Proposal

  • bin/oil can be identical to bin/osh in this regard: a = 42 is NOT special
  • LATER: bin/oven is for configuration, and has shopt -s parse_equals, which allows a = 42 as an alias for const

So does that mean we need shopt -s oven:all or something?

Does it enforce some kind of sandboxing? I think it can change the "first word lookup" #453

Maybe command ls is REQUIRED in this case

@andychu
Copy link
Contributor Author

andychu commented Apr 3, 2022

Another Idea: Only allow it within blocks ? #631

So it's disallowed at the top level and within procs.

But we could silently turn the option on for

server www.example.com { 
  port = 80
}

However then it's also on in

shopt --unset errexit {
  ls /
  ls / bin
  x = 42
}

That's perhaps confusing


I wonder if we can do

shopt --set parse_equals {
  server www.example.com { 
    port = 80
  }
}

Is that awkward?

@andychu
Copy link
Contributor Author

andychu commented Apr 30, 2022

Another issue: const has static checks within proc.

So maybe we should

  • turn off parse_equals in bin/oil. Save it for bin/oven and some kind of eval --config (myfile) (eval builtin can take a block #1025 )
  • and also remove the static check. So you can do:
proc myrules(name) {

  rule "$name-python" {
     kind = 'python'
  }

  rule "$name-cc" {
     kind = 'cc'
  }

}

The kind variable should not conflict! The proc can be used for parameterization, like

myrules foo
myrules bar

andychu pushed a commit that referenced this issue Apr 30, 2022
This addresses #937, which was also noted by a user.

Bare assignment like 'x = 42' was equivalent to 'const x = 42'.

However this causes inconsistency in programs.  We only want bare
assignment for config dialects like:

    server www.example.com {
      root = '/'
      port = 8080
    }

On the other hand we don't want

    proc p {
      const x = 42
      y = 43  # inconsistent
    }

Also add a failing test for bare assignment in blocks.  This is another
issue -- some blocks will introduce a new scope, and some won't.

So we don't want the static check for 'const already defined in scope'.
Instead bare assignment will only do DYNAMIC checks (similar to the top
level), and 'const' will do STATIC checks.
andychu pushed a commit that referenced this issue Apr 30, 2022
Unlike 'const', it doesn't do static checks for 'const' already defined.

Updated doc/variables.

Related to #937.
andychu pushed a commit that referenced this issue Apr 30, 2022
Related to #937.

Fixed a syntax error that wasn't hidden behind parse_equals!
@andychu
Copy link
Contributor Author

andychu commented May 6, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant