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

v7.1.2 doesn't parse --file=message.txt kind of tag argument correctly #179

Closed
ruohola opened this issue Apr 19, 2021 · 4 comments · Fixed by #180
Closed

v7.1.2 doesn't parse --file=message.txt kind of tag argument correctly #179

ruohola opened this issue Apr 19, 2021 · 4 comments · Fixed by #180
Assignees
Labels
type: bug Verified problems that need to be worked on
Projects

Comments

@ruohola
Copy link

ruohola commented Apr 19, 2021

When using:

uses: EndBug/add-and-commit@v7
with:
  tag: 'v1.0.0 --annotate --file=message.txt'

the last argument gets parsed as --file =message.txt, when it should be parsed to just --file=message.txt.

This results in the following error:

> Tagging commit...
##[error]Error: fatal: could not open or read '=message.txt': No such file or directory

##[error]Error: fatal: could not open or read '=message.txt': No such file or directory

> Pushing commit to repo...

Demo of the buggy parsing

Caused by #174 which fixed another bug and created this regression.

@ruohola ruohola added the status: pending More info is needed before deciding what to do label Apr 19, 2021
@EndBug EndBug added the type: bug Verified problems that need to be worked on label Apr 19, 2021
@EndBug EndBug self-assigned this Apr 19, 2021
@EndBug EndBug removed the status: pending More info is needed before deciding what to do label Apr 19, 2021
@EndBug EndBug added this to To do in Main board via automation Apr 19, 2021
@EndBug
Copy link
Owner

EndBug commented Apr 19, 2021

Hi, I've done some research, and this is what I've found.

The process.argv array is not generated by Node, but by the shell / OS, and so there's no way for me to recreate its exact behavior.

The same guy that made the core of the previous function made some adjustments, which I then turned in this function. It should parse pretty much every case and generate an arg array valid for simple-git.
This is the interactive demo.

This is fine, but it has two main issues:

  • It's far from being perfect, and there will always be a case where a command works on your machine but doesn't with the action, because of the radical difference of the parsing
  • It's not the easiest thing to maintain: it relies on a Regex which I frankly don't understand very well, and the fact that I didn't code its core doesn't make it better. If it needs improvement it's not going to be easy

The ideal solution would be to rely on string-argv, which seems to do the job well but doesn't correctly match --message "This is \"a\" 'quoted' message" (demo).
I'll see if I can get to use this, maybe even dropping support for those type of strings.

I dunno, I'll think about it.

@ruohola
Copy link
Author

ruohola commented Apr 20, 2021

I dunno, I'll think about it.

No hurry at least in my end :) I can manage fine with either v7.1.1 or just by not using the entirely optional = between the argument name and the value.

Main board automation moved this from To do to Done Apr 21, 2021
@EndBug
Copy link
Owner

EndBug commented Apr 21, 2021

I've ended up deciding to rely on string-argv, just because it's way easier to maintain. I've also added a paragraph about argument parsing in the README, so that anybody having problems may find their answer.
Thanks a lot for your contributions regarding this topic ❤️

@EndBug
Copy link
Owner

EndBug commented Apr 22, 2021

ℹ️ Fix published in v7.2.0 (also v7 and latest)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Verified problems that need to be worked on
Projects
No open projects
Main board
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants