Skip to content

Avoid conflict with run() function with another bash frameworks #630

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

Merged
merged 2 commits into from
Apr 22, 2025

Conversation

vladriabtsev
Copy link
Contributor

@vladriabtsev vladriabtsev commented Apr 18, 2025

When I am trying to use 'bashmatic' framework for my new bash script, run() function from this framework is conflicting with run() function from bashly generated script

@DannyBen
Copy link
Member

Thanks for the contribution. I do have a few concerns, mainly around the potential for breaking changes - this could have a significant impact. Also, if I’m remembering correctly, run is intended to be available for users to make internal calls as well.

I'm curious about the reasoning behind mixing frameworks here — could you clarify what Bashmatic brings to the table in this context? If there's a use case that truly requires this approach and can’t be addressed in a less disruptive way, I’d definitely be open to exploring it further.

@DannyBen DannyBen marked this pull request as draft April 18, 2025 17:05
@DannyBen
Copy link
Member

I pushed a different solution - simply allow customizing the names of some internal functions. Currently, only initialize and run but this is easily extensible.

It has the advantages of a) being explicit, b) maintaining backwards compatibility.

Thoughts?

@vladriabtsev
Copy link
Contributor Author

I am beginner in writing bash scripts. I have created draft installer script with Bashmatic as framework for UI. I decided to use Bashly to create better parameters for my script.

Majority Bashmatic functions are grouped and have unique names inside a group. Typical Bashmatic function name is 'group-name.function-name()'. It is helping avoid conflicts when such framework are sourced. At least 'run()' function is exception. It is without group.

Do you know better solution how use several frameworks together and avoid conflicts with function names?

@DannyBen
Copy link
Member

DannyBen commented Apr 21, 2025

Thanks for the additional information.

While bashly is designed to be the primary foundation, you should be able to add functions from other frameworks to it. Here are my thoughts, in general (as I am not fully familiar with bashmatic).

  1. Generally speaking, I would try to avoid mixing frameworks - unless, the frameworks are providing solutions for different problems. In the bashly-bashmatic case, I am not sure if this case fits (I would love to be enlightened one way or the other).
  2. To my understanding, the main problem you encounter is that the function run is used by bashmatic code snippets and other functions to run commands inside a wrapper function. This makes sense, and even without bashmatic in the picture, some people would like to reserve the function name run for other uses.
  3. I accept the fact that some internal functions - namely run and initialize - may conflict with other needs.
  4. The updated code I pushed to the pull request, addresses this change in a different way - instead of being implicit and not backwards compatible, it is explicit: If someone wants different names, they can easily do so by using bashly settings ($ bashly add settings to get the full file).

If this makes sense, I can merge it.

@vladriabtsev
Copy link
Contributor Author

I pulled your latest changes. Added next lines in my settings file:
function_names:
run: bashly-run
initialize: bashly-initialize

After rebuild an install modified gem I am getting next error when 'bashly generate' for my project:
Psych::SyntaxError
(): did not find expected key while parsing a block mapping at line 18 column 1

Visual Studio Code is showing next error in my settings file:
Property function_names is not allowed. [yaml-schema: Bashly Settings (bashly-settings.yml)(513)]

Probably modified yaml-schema file is not committed yet.

@DannyBen
Copy link
Member

The schema was not yet updated, but it shouldn't matter.

Your YAML is inorrect, you need indentation and bash function names should not contain a hyphen.

function_names:
  run: bashly_run
  initialize: bashly_initialize

@DannyBen DannyBen marked this pull request as ready for review April 22, 2025 10:48
@DannyBen
Copy link
Member

I am going to merge this, as I want to work on the schema, and perhaps other related changes. I believe this should solve your use case easily - if you need help, feel free to open a discussion or issue. Thanks for bringing this to our attention.

@DannyBen DannyBen merged commit d95c5de into bashly-framework:master Apr 22, 2025
7 of 8 checks passed
@DannyBen DannyBen added this to the 1.2.12 milestone Apr 22, 2025
@DannyBen DannyBen changed the title avoid conflict with run() function with another bash frameworks Avoid conflict with run() function with another bash frameworks Apr 22, 2025
@DannyBen
Copy link
Member

DannyBen commented May 2, 2025

This is released now in v1.2.12.

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

Successfully merging this pull request may close these issues.

2 participants