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

Last argument 'pop' (like shift) #67

Open
coderofsalvation opened this issue Jun 6, 2020 · 8 comments
Open

Last argument 'pop' (like shift) #67

coderofsalvation opened this issue Jun 6, 2020 · 8 comments

Comments

@coderofsalvation
Copy link
Owner

Foo(){
  shift;
  pop; # does not work grr
}

Foo removeme abc removeme
@fcard
Copy link
Collaborator

fcard commented Sep 7, 2020

Foo() {
  shift
  set -- "${@:1:$#-1}" # Pop
  echo "$@"
}
Foo removeme abc removeme # abc

Here's a posix way, though it's very hacky

Foo() {
  shift

  # Pop
  local index=0
  local arguments=""
  while [ $index -lt $(($#-1)) ]; do
    index=$((index+1))
    arguments="$arguments \"\$$index\""
  done
  eval "set -- $arguments"

  echo "$@"
}
Foo removeme abc removeme # abc

This could indeed be implemented as a pop keyword in powscript. I will do it tomorrow! I need to get back to this.

@coderofsalvation
Copy link
Owner Author

fcard!
Interesting, you're using 'set' to overwrite the positioned variables?

@fcard
Copy link
Collaborator

fcard commented Sep 7, 2020

Hello! It's been a while.

And yes, set -- x y z sets $1 to x, $2 to y, and $3 to z, with the -- preventing any of the arguments from being interpreted as options, otherwise if an argument was "-e" or something it would fail. That's the only way I could think of to implement this pop functionality, unfortunately there isn't a way to set a specific positional argument that I know of.

I am going to give it a try implementing this in a moment. Thankfully it seems like I still remember most of the details about this codebase!

@fcard
Copy link
Collaborator

fcard commented Sep 8, 2020

I've been trying to get the Posix version to be faster, and this is what I've come up:

PopSet() {
  local N=$(($1 - ${2:-0}))
  local index=0 i1 i2 i3 i4 i5 i6 i7 i8 i9
  local arguments=""
  case $N in
    1) _POP_ARGS='';;
    2) _POP_ARGS='"$1"' ;;
    3) _POP_ARGS='"$1" "$2"' ;;
    4) _POP_ARGS='"$1" "$2" "$3"' ;;
    5) _POP_ARGS='"$1" "$2" "$3" "$4"' ;;
    6) _POP_ARGS='"$1" "$2" "$3" "$4" "$5"' ;;
    7) _POP_ARGS='"$1" "$2" "$3" "$4" "$5" "$6"' ;;
    8) _POP_ARGS='"$1" "$2" "$3" "$4" "$5" "$6" "$7"' ;;
    9) _POP_ARGS='"$1" "$2" "$3" "$4" "$5" "$6" "$7" "$8"' ;;
    *)
      while [ $index -lt $((N-9)) ]; do
        i1=$((index+1))
        i2=$((index+2))
        i3=$((index+3))
        i4=$((index+4))
        i5=$((index+5))
        i6=$((index+6))
        i7=$((index+7))
        i8=$((index+8))
        i9=$((index+9))
        arguments="$arguments \"\$$i1\" \"\$$i2\" \"\$$i3\" \"\$$i4\" \"\$$i5\" \"\$$i6\" \"\$$i7\" \"\$$i8\" \"\$$i9\""
        index=$i9
      done
      case $((N%9)) in
        2) arguments="$arguments \"\$$i1\"" ;;
        3) arguments="$arguments \"\$$i1\" \"\$$i2\"" ;;
        4) arguments="$arguments \"\$$i1\" \"\$$i2\" \"\$$i3\"" ;;
        5) arguments="$arguments \"\$$i1\" \"\$$i2\" \"\$$i3\" \"\$$i4\"" ;;
        6) arguments="$arguments \"\$$i1\" \"\$$i2\" \"\$$i3\" \"\$$i4\" \"\$$i5\"" ;;
        7) arguments="$arguments \"\$$i1\" \"\$$i2\" \"\$$i3\" \"\$$i4\" \"\$$i5\" \"\$$i6\"" ;;
        8) arguments="$arguments \"\$$i1\" \"\$$i2\" \"\$$i3\" \"\$$i4\" \"\$$i5\" \"\$$i6\" \"\$$i7\"" ;;
        0) arguments="$arguments \"\$$i1\" \"\$$i2\" \"\$$i3\" \"\$$i4\" \"\$$i5\" \"\$$i6\" \"\$$i7\" \"\$$i8\"" ;;
      esac
      _POP_ARGS="$arguments"
      ;;
  esac
}

Foo() {
  PopSet $#
  eval "set -- $_POP_ARGS"
  echo "$@"
}

Foo a b c # a b

This is about as fast on dash as bash's pop is on bash, which isn't too shabby. Neither of them scale as well as shift when being called multiple times in a function though, so putting it in a loop could slow down scripts. That said, unsetting the last element of an array is O(1) and it's not too hard to do right now:

Foo(@args)
  local length=${args[@]:length}
  unset args[$length-1]
  echo $args[@]

Foo a b c # a b

In addition to the positional arguments pop we could add an array:pop and an array:shift as well that could be used like this:

Foo(@args)
  array:shift args
  array:pop args
  echo "$@"

Foo a b c # b

They could be implemented in the lib/std.pow file simply as

array:shift(A)
  expand
    ~A=(${~A[@]:slice 1})

array:pop(A)
  expand
    unset ~A[${~A[@]:length}-1]

(The issue with using arrays of course is that while pop becomes O(1), shift becomes O(n). weh!)

@coderofsalvation
Copy link
Owner Author

coderofsalvation commented Sep 8, 2020

interesting!
I can see you were in a creative mood :)
The string-composition using case seems to indicate that for-loops were too expensive?
I really like the usage of array:shift and array:pop as they are also useful outside of the pop/push scenario.

@fcard
Copy link
Collaborator

fcard commented Sep 8, 2020

The initial case statement is simply because I assume that most function calls won't have >9 arguments, so we simply go to the result, and in the case of a large amount of arguments, a case statement is such a light thing it won't matter in the end.

For the rest, the main issue is that string concatenation is expensive, specially if you have many small operations, so I figured the best way to cut out the expense was to concatenate the indexes in batches; I picked 9 as a good not too large not too small number, and it was enough to match the bash version in speed in most of my benchmarks. Though, only in dash, in bash it's still 2-4x slower than just using the sub-array method, so we should still use it there.

@fcard
Copy link
Collaborator

fcard commented Sep 17, 2020

Sorry for the break, I was doing research on this pop predicament, which I've compiled here: https://github.com/fcard/pop.sh
I am also busy with a game project, so I will be multitasking between this and that, but I still want to come back to this proper, nyeh
Expect a pull request a few hours from now!

@coderofsalvation
Copy link
Owner Author

coderofsalvation commented Sep 18, 2020 via email

@fcard fcard mentioned this issue Sep 18, 2020
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

2 participants