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

split functions #961

Closed
hohwille opened this issue Oct 25, 2022 · 0 comments · Fixed by #934
Closed

split functions #961

hohwille opened this issue Oct 25, 2022 · 0 comments · Fixed by #934
Labels
bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) enhancement New feature or request functions related to the collection of bash functions in functions[-core] file

Comments

@hohwille
Copy link
Member

hohwille commented Oct 25, 2022

In devonfw-ide we have created reusable functions in a file called functions.
We also have 2 special functions in environment-project for more global reuse.
This is because when you just call devon without arguments the script can not be executed as a sub-process as it needs to "tweak" variables like PATH of the current shell.
If we would source the functions here, we would pollute the callers shell with tons of functions (including side-effects and seeing them in tab-completion, etc.).
Therefore we only source environment-project.
To avoid redundancies environment-project defines these 2 functions that are also reused in commandlets and other functions.
Therefore they are declared in environment-project but when devon is called without arguments, it will afterwards "unload" these 2 functions to remove them from the callers shell.
The commandlets source functions that again sources environment-project.

Now, with PR #934 and issue #878 we have extended some cross-cutting functionality executed in functions.
Also we have to define some things in commandlets before we source functions.
If in this case we want to use some function like e.g. doIsWindows or doIsMacOs we can not do so as the function is not yet defined at that point.
As a result so far I have copied the logic from such functions into the commandlet.
It is about time to rethink this approach and come up with a clean design resolving this problem.
IMHO we should have two files that only declare functions and have (almost) no side effects:

  • functions-core
    (if someone has a better name for this, please go ahead)
  • functions
    (only side effects: source functions-core, handle DEVON_IDE_TRACE, set DEVON_IDE_HOME)

The side effects should happen in the following files:

  • environment-project
    (sources functions-core and then does what it already does currently except declaring functions what is moved to functions-core)
  • commandlet-cli
    (here we extract the current side-effects of functions what is at the bottom of the file and parsing CLI paramters)

As a result a commandlet header will look like this:

#!/usr/bin/env bash

# autocompletion list
if [ "${1}" = "shortlist" ]
then
  ...autocompletion here...
  exit
fi

# the following line will be moved to the end of functions and removed from all commandlets
#if [ -n "${DEVON_IDE_TRACE}" ]; then set -vx; fi
# shellcheck source=scripts/functions
source "$(dirname "${0}")"/../functions
«TOOL»_HOME="${DEVON_IDE_HOME}/software/«tool»"
... here we can already use any function
TOOL_VERSION_COMMAND=...
#override functions such as doResolveDownloadUrl here
# shellcheck source=scripts/commandlet-cli
source "$(dirname "${0}")"/../commandlet-cli
... rest of the commandlet with doSetup, cli-parsing, etc. ...
@hohwille hohwille added enhancement New feature or request bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) Team_IDE functions related to the collection of bash functions in functions[-core] file labels Oct 25, 2022
hohwille added a commit to hohwille/ide that referenced this issue Oct 25, 2022
hohwille added a commit to hohwille/ide that referenced this issue Oct 25, 2022
@hohwille hohwille added this to the release:2022.08.004 milestone Oct 31, 2022
hohwille added a commit that referenced this issue Nov 7, 2022
… doInstall (#934)

* #878: insteall latest version if unspecified

* #933: fixed typo

* #878: insteall latest version if unspecified also for docker

* #878: added to CHANGELOG

* #826: excuse rule for cobigen added

* #878: fixed stupid bug

* #847: fix for global tools and cobigen

* #878: #933: fixes and improvements

* #893: ability to configure version prefix

* #940: handle silent mode in doInstall and simplify doSetup

* #934: shellcheck fix

* #934: CHANGELOG

* #940: generalize macos workarounds

* #940: generalize macos workarounds

* #940: generalize macos workarounds

* #940: need to implement on mac, small improvements for debugging

* #940: generalize macos workarounds

* #940: generalize macos workarounds

* #940: fix - thanks shellcheck

* #940: fixed doInstall java extra arg order

* #940: OMG: fixed custom tools bug

* #893: extended test and fixed doVersionCompare

* #934: removed code entirely

* #934: removed TODO

* #934: simplified

* #934: fixed download caching

* #934: fixed aws

* #934: keep code to remove aws installer

* #934: fixed typos

* #957: symlink fix

* #957: symlink fix

* #934: fixed gcviewer

* #934: shellcheck fix

* #934: shellcheck war starts

* #934: improve DEVON_SOFTWARE_PATH on win

* #934: shellcheck fix

* #958: make symlinks work on windows

* #958: updated documentation

* #934: improved DoD from review comment

* #934: define HOME always on top

* #960: prevent "oc version" error

* #940: improve MacOS workaround

* #934: fixed doMavenArchetype

* #934: fixed aws

* #934: aligned and simplified

* #940: fixes for MacOS workaround

* #940: MacOS quickfix for eclipse

* #911: further improvements for eclipse

* #934: shellcheck fix

* #934: nasty shellcheck

* #960: added to CHANGELOG

* #961: split functions, fixed intellij version command on mac

* #961: fixed function doc

* Fix Add-plugin and custom start parameter

I fixxed the add plugin function which forgot to call vscode with "--install-extension".
And I changed the run command so it passes the arguments given to the run method, so you can run vscode with custom start parameters aswell now.

For example:
devon vscode run --list-extension

* #934: removed debug output

* #961: fixed function doc

* #934: #943: improve vscode plugin installation

* rename pip-latest-windows to pip-latest-pip

There was in an error installing pip on Windows because the installation file was renamed from pip-latest-windows-py to pip-latest-pip.py

* use cg --version

* #934: added missing X flags

* #940: fixed MacOS Workaround

* #934: python fix due to doInstall installer missuse

* #934: improved doc for shared software

* #934: rancher fix due to doInstall installer missuse

* #934: kubectl shall not have knowledge about docker installation

Co-authored-by: Genetics <102921542+Amueller36@users.noreply.github.com>
Co-authored-by: alfeilex <101652401+alfeilex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) enhancement New feature or request functions related to the collection of bash functions in functions[-core] file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant