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

[Feature] add option to return forward slashes on windows instead of backslash #2044

Closed
4 tasks done
rantoniuk opened this issue Apr 15, 2024 · 4 comments · Fixed by #2056
Closed
4 tasks done

[Feature] add option to return forward slashes on windows instead of backslash #2044

rantoniuk opened this issue Apr 15, 2024 · 4 comments · Fixed by #2056
Labels
enhancement New feature or request

Comments

@rantoniuk
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Does this issue exist in the latest version?

  • I'm using the latest release

Describe the bug?

It seems like a regression for #340. I have this:

jobs:
  tests:
      runs-on: windows-latest
      name: tests

...

        - name: Get changed files
          id: changed-files
          uses: tj-actions/changed-files@v44
          with:
            safe_output: false
            files_ignore: |
              segmentation/**
            files: |
              **/*.py
              **/*.pyi

        - name: List all changed files
          shell: bash
          env:
            ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
          run: |
            for file in ${ALL_CHANGED_FILES}; do
              echo "$file was changed"
            done

...

        - name: "Code Lint - Black"
          uses: psf/black@stable
          if: steps.changed-files.outputs.all_changed_files
          env:
            ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
          with:
            src: ${{ env.ALL_CHANGED_FILES }}

...

When ran on a windows runner, I see backslashes for directories instead of forward slashes in List all changed files output. Then the black job fails because it misinterprets the backslashes as escapes on Windows.

To Reproduce

As above.

What OS are you seeing the problem on?

windows-latest or windows-2022

Expected behavior?

Forward slashes should be used always in the output - see e.g.
https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#include

Relevant log output

Too complex workflow to push all the debug logs.

Has all relevant logs been included?

  • I've included all relevant logs

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@rantoniuk rantoniuk added the bug Something isn't working label Apr 15, 2024
@tj-actions-bot
Copy link
Contributor

Thanks for reporting this issue, don't forget to star this project if you haven't already to help us reach a wider audience.

@rantoniuk rantoniuk changed the title [BUG] <title> forward slashes should be used for directories Apr 15, 2024
@jackton1 jackton1 changed the title forward slashes should be used for directories [Feature] Add support for returning forward slashes on windows Apr 15, 2024
@jackton1 jackton1 added bug Something isn't working enhancement New feature or request and removed bug Something isn't working labels Apr 15, 2024
@rantoniuk
Copy link
Author

@jackton1 can you explain why do you see this as a feature and not a bug? For me this is effectively a bug, as the paths on windows runners are formatted incorrectly and need to be postprocessed to replace \ with / so that the later steps can consume them properly.
Am I missing something?

@rantoniuk rantoniuk changed the title [Feature] Add support for returning forward slashes on windows [Feature] Use forward slashes in paths by default Apr 16, 2024
@jackton1 jackton1 changed the title [Feature] Use forward slashes in paths by default [Feature] add option to return forward slashes on windows instead of backslash Apr 17, 2024
@jackton1
Copy link
Member

jackton1 commented Apr 17, 2024

@rantoniuk Windows operating systems use the backslash \ as the folder path separator. This is in contrast to Unix-based operating systems, such as Linux and macOS, which use the forward slash / as the folder path separator. As such this doesn't qualify as a bug because others running on Windows expect a backslash folder path separator.

@jackton1
Copy link
Member

jackton1 commented Apr 18, 2024

HI @rantoniuk, you can now set the use_posix_path_separator input to true on Windows to get the POSIX path separator / using the https://github.com/tj-actions/changed-files/releases/tag/v44.3.0

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 a pull request may close this issue.

3 participants