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

Plugin/Java: quote path and provide useful error message #2006

Merged
merged 3 commits into from
Jan 1, 2022

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Dec 30, 2021

Description

Quote the path to the JAR file in jar_manifest(), and provide a meaningful error message if no path is provided.

Motivation and Context

Just cleaning up; this function would fail for any files with spaces in the path. Alsö, shfmt.

Note: this branch alsö includes a fix to path quoting in test/run; otherwise tests wouldn't be able to run at all when $BASH_IT has a space in it.

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

Are these files related to the PR?:

  • test/plugins/xterm.plugin.bats
  • test/run

@gaelicWizard
Copy link
Contributor Author

Are these files related to the PR?:

  • test/plugins/xterm.plugin.bats
  • test/run

Yes, I can't run BATS at all without fixing unquoted paths, so I can't run the test suite before submitting without those two fixes. I suppose I could pull them from this PR if you prefer

@davidpfarrell
Copy link
Contributor

Yes, I can't run BATS at all without fixing unquoted paths, so I can't run the test suite before submitting without those two fixes. I suppose I could pull them from this PR if you prefer

Sounds like you want a separate PR to address running bats in environments that require quoted paths ...

@gaelicWizard
Copy link
Contributor Author

Yes, I can't run BATS at all without fixing unquoted paths, so I can't run the test suite before submitting without those two fixes. I suppose I could pull them from this PR if you prefer

Sounds like you want a separate PR to address running bats in environments that require quoted paths ...

I did. It's one of my two dozen open PRs. I'll pull those two commits from this PR if you prefer

@davidpfarrell
Copy link
Contributor

I did. It's one of my two dozen open PRs. I'll pull those two commits from this PR if you prefer

Can you reference it? I took a quick look at open PRs and didn't recognize it (by name) ...

We can consider this a draft and get that other PR merged at which point it should disappear from this PR ...

Provides an error message if no file is specified.
Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

Approved !

@gaelicWizard
Copy link
Contributor Author

gaelicWizard commented Dec 31, 2021

Can you reference it? I took a quick look at open PRs and didn't recognize it (by name) ...

My BATS fixes are currently consolidated under #2002.

EDIT: and has been part of my shellcheck branch (#1917) since October 19th.

@NoahGorny NoahGorny merged commit c5d3b25 into Bash-it:master Jan 1, 2022
@gaelicWizard gaelicWizard deleted the plugin-java branch January 1, 2022 21:56
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.

None yet

3 participants