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

Diminish the notion of prelude #84

Closed
xonixx opened this issue Nov 27, 2021 · 0 comments
Closed

Diminish the notion of prelude #84

xonixx opened this issue Nov 27, 2021 · 0 comments
Assignees
Milestone

Comments

@xonixx
Copy link
Owner

xonixx commented Nov 27, 2021

Discovered in #81 (comment)

What

Disallow arbitraty shell scripts in prelude area (i.e. before goals declaration).

Now allowed

if [[ "$OSTYPE" == "linux-gnu"* ]]; then
  @define os="linux"
elif [[ "$OSTYPE" == "darwin"* ]]; then
  @define os="darwin"
fi
@define URL="https://github.com/file-$os.tar.gz"
@goal download
  wget "$URL"  

Proposed

Only allow @define,@shell,@options and comment lines in prelude area, no plain shell code

Why

  1. This feature is not widely used yet and even have implementation bugs not noticed for a long time.
  2. We don't know yet how to use this properly. It poses a surface to be abused/misused.
  3. Introduces ambiguity. If we need such complex initialization logic, shall we use instead @goal initialized?
  4. It complicates implementation and makes it less efficient by relying on temp files.

How

We change the implementation to limit the flexibility of prelude or remove this notion altogether.

Need not forget to adjust README accordingly.

This could be a breaking change, need to find a way to cause least damage.

Things to consider

  • Syntax:
    • @define VAR='hello' (as now) vs
    • @define VAR 'hello' (more consistent with other directives syntax)
    • Allow or disallow double-quoted strings? This means do we support or no variables substitution like:
      • @define W=world
      • @define HW="Hello $W!"
  • Implementation
    • pass-thru to shell (as now)
      • Flexibility with variables substitution but less validation
    • awk-level parsing
      • Harder implementation, more control of validation of uninitialized vars, disallowed shell features for define, like @define A="$(curl https://google.com)"

Resolution

For release 0.9.15 let's proceed with gradual approach. We leave the current syntax @define VAR="Value $OTHER_VAR" but we remove the ability to run arbitrary shell in prelude area.
Later we need to reconsider the syntax to make parsing more strict and unified, to disallow things like #85.

@xonixx xonixx self-assigned this Nov 27, 2021
xonixx added a commit that referenced this issue Dec 8, 2021
xonixx added a commit that referenced this issue Dec 8, 2021
xonixx added a commit that referenced this issue Dec 8, 2021
xonixx added a commit that referenced this issue Dec 8, 2021
@xonixx xonixx added this to the PLANNED milestone Dec 8, 2021
@xonixx xonixx mentioned this issue Dec 8, 2021
@xonixx xonixx modified the milestones: PLANNED, 0.9.15 Dec 8, 2021
@xonixx xonixx closed this as completed Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant