Skip to content

[WIP] remove shopt&alias to make it work on powershell#131

Closed
pvgenuchten wants to merge 1 commit intogeopython:masterfrom
pvgenuchten:remove_alias_for_powershell
Closed

[WIP] remove shopt&alias to make it work on powershell#131
pvgenuchten wants to merge 1 commit intogeopython:masterfrom
pvgenuchten:remove_alias_for_powershell

Conversation

@pvgenuchten
Copy link
Copy Markdown

@pvgenuchten pvgenuchten commented Jun 21, 2023

PR to facilitate discussion on this topic

docker-compose was removed >1 year ago, i think we can assume all are migrated by now?

removing shopt and alias because it is not supported on powershell/windows

todo: update docs for windows:
syntax: bash geopython-workshop-ctl.sh start|stop|url|update

  • works on powershell (windows+docker desktop)
  • works on ubuntu (WSL)

@pvgenuchten pvgenuchten requested a review from justb4 June 21, 2023 09:58
@tomkralidis tomkralidis changed the title remove shopt&alias to make it work on powershell [WIP] remove shopt&alias to make it work on powershell Jun 21, 2023
@pvgenuchten pvgenuchten force-pushed the remove_alias_for_powershell branch from ae4fa34 to fa0fc91 Compare June 21, 2023 11:25
@pvgenuchten
Copy link
Copy Markdown
Author

@tomkralidis, updated docs for powershell

@pvgenuchten pvgenuchten requested a review from tomkralidis June 21, 2023 11:30
Copy link
Copy Markdown
Member

@justb4 justb4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, looks like these changes remove the alias and assume the command is docker compose. This will break for students having docker-compose installed (like me).

Best option is to make the script cross-platform. The $IsWindows var e.g.
and https://redmondmag.com/articles/2020/08/18/cross-platform-powershell-scripts.aspx

@justb4 justb4 added the enhancement New feature or request label Jun 21, 2023
@justb4 justb4 added this to the FOSS4G 2023 milestone Jun 21, 2023
@pvgenuchten
Copy link
Copy Markdown
Author

pvgenuchten commented Jun 21, 2023

ah, good to know people are still using docker-compose

alternative would be to duplicate the script and rename to _ps.sh

@justb4 PR updated

@pvgenuchten pvgenuchten force-pushed the remove_alias_for_powershell branch from fa0fc91 to 6426d43 Compare June 21, 2023 14:20
syntax:  bash geopython-workshop-ctl.sh start
@pvgenuchten pvgenuchten force-pushed the remove_alias_for_powershell branch from 6426d43 to 1b625d8 Compare June 21, 2023 14:24
docker compose rm --force
elif [ $1 == "url" ]; then
# try to open the Jupyter Notebook in Browser
platform="$(uname | tr '[:upper:]' '[:lower:]')"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still required? I guess the openapp="cmd /c start" is the command in PS here.

@justb4
Copy link
Copy Markdown
Member

justb4 commented Jun 22, 2023

I cannot test this, but seems useful. I wonder if the uname stuff even works. I have no idea how Powershell relates to WSL and Docker install. I have the feeling we would not need a separate Bash script with some smart 'sniffing' like the $Windows var and uname command, branching in the original script. It may be confusing even, two scripts with almost identical names.

@tomkralidis
Copy link
Copy Markdown
Member

Bumping @pvgenuchten in case we want to add this for FOSS4 EU 2025.

@pvgenuchten
Copy link
Copy Markdown
Author

superseded by #203

@pvgenuchten pvgenuchten closed this Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants