Skip to content

cqfd: do not handle semicolon comments#141

Open
gportay wants to merge 1 commit intosavoirfairelinux:masterfrom
gportay:stops-handling-comment
Open

cqfd: do not handle semicolon comments#141
gportay wants to merge 1 commit intosavoirfairelinux:masterfrom
gportay:stops-handling-comment

Conversation

@gportay
Copy link
Copy Markdown
Contributor

@gportay gportay commented Feb 2, 2025

The .ini files uses the semicolon for comments. Also, that exact same character is one of the shell control operator for list of pipelines.

The hacky parser is not smart enough to make the distinction if a semicolon is an .ini comment or a shell control operator.

It considers any semicolon character as a .ini comment, and it removes the remaining characters till the EOL (including the semicolon), even if they are part of a list of pipeline.

As a consequence, it cuts off the list of pipelines if the command= values contain a semicolon character, and it results in running the left-hand only.

This stops to handle the semicolon comments to avoid amputating the list of pipelines controled by semicolons that are set in values of the command= properties.

Note: The parser converts the .ini file to bash; as a consequence, the shell comments do not affect the parsing, as long as the hashmark does not break the syntax AND if it not put at the end of the section line.

@gportay gportay force-pushed the stops-handling-comment branch from 78c667b to f440a55 Compare February 2, 2025 11:49
@florentsfl
Copy link
Copy Markdown
Contributor

So there is no way to add comments to a cqfdrc file ? And this breaks if someone has a comment in one of his cqfd file ?

We may bump major version after that

@gportay
Copy link
Copy Markdown
Contributor Author

gportay commented Feb 15, 2025

So there is no way to add comments to a cqfdrc file ?

There is a way to add comments... since the .ini file is transformed into shell, the shell comment (i.e. starting after a #) works.

The #-style comment works before and after that PR is applied:

gportay@archlinux ~/src/cqfd $ cat .cqfdrc 
[project]
org='fooinc'
name='barproject'

[hello]
command="echo 'Hello World!'"

[app]
command='./configure && make'

[debug]
command='./configure --enable-debug && make DEBUG=all'

# comment was working outside a section
[build]
# comment was working inside a section
command='cp README.md README.FOOINC \
         && asciidoc README.FOOINC'
files='README.html'
archive='cqfd_${BUILD_ID}_%Gh.tar.xz'
gportay@archlinux ~/src/cqfd $ bash cqfd run whoami
gportay

And this breaks if someone has a comment in one of his cqfd file ?

Yes, if someone use the ; to comment things.

The ;-style comment was working before that PR:

gportay@archlinux ~/src/cqfd $ cat .cqfdrc 
[project]
org='fooinc'
name='barproject'

[hello]
command="echo 'Hello World!'"

[app]
command='./configure && make'

[debug]
command='./configure --enable-debug && make DEBUG=all'

; comment was working outside a section
[build]
; comment was working inside a section
command='cp README.md README.FOOINC \
         && asciidoc README.FOOINC'
files='README.html'
archive='cqfd_${BUILD_ID}_%Gh.tar.xz'
gportay@archlinux ~/src/cqfd $ bash cqfd run whoami
gportay

Add stops working if that PR is applied:

gportay@archlinux ~/src/cqfd $ bash cqfd run whoami
cqfd: fatal: /home/gportay/src/cqfd/.cqfdrc: Invalid ini-file!

Important: The worse thing with the parse is you cannot comment in the first file of the file cqfdrc, because of that. Comment can be added everywhere, including the last line.

@gportay
Copy link
Copy Markdown
Contributor Author

gportay commented Feb 15, 2025

We may bump major version after that

I guess your are right, it is preferable to bump to cqfd 6; and maybe we can apply more breaking changes at the same time. And we should also post a note in the README.md file.


I am thinking about deprecating the command run in favor of cqfd shell or cqfd exec (and I do not know how to deal with run -c?). Anyway, I really like your opinion on that.

So if we go to depreciate run, and if we do not care about breaking the CLI, I am thinking about the sudoisation (first commit) for cqfd 6:

  1. cqfd run foo bar baz and cqfd exec foo bar baz become cqfd foo bar baz
  2. cqfd run "foo bar baz; qnux" and cqfd shell -c "foo bar baz; qnux" become cqfd sh -c "foo bar baz; qnux"
  3. cqfd shell becomes cqfd sh
  4. cqfd bash remains cqfd bash see 1.
  5. cqfd [-b flavor] still run [flavor.]command=
  6. #/usr/bin/env -S cqfd shell is #!/usr/bin/env -S cqfd sh see 3.
  7. cqfd run -c "&& echo foo bar baz" ??? maybe cqfd -c "&& echo foo bar baz"
  8. cqfd run -c --foo --bar --baz ??? maybe invalid, and should be cqfd -c "--foo --bar --baz" instead.

In short, run, exec, shell verbs disappear; the CLI is simplified to cqfd [-f .cqfdrc] [-d .cqfd] [-C chdir] [-q] [-b flavor] [--verbose] [--init] [--release] [command_name [command_argument]]. cqfd remains cqfd, cqfd ... runs cqfd exec ....

This is an idea I have in mind.

But that is not the right place to talk about that. It is better to file an issue if this is something that should be considered. This sounds to need a plan for merging the different PRs to prepare a 5.7.0 (i.e. probably all the PRs up to 150, excluding that one about the comment). And then applies that one and rework the first commit of #106 for cqfd 6.

@gportay gportay force-pushed the stops-handling-comment branch 2 times, most recently from 0eb149f to d5e9a7f Compare February 16, 2025 10:35
@florentsfl
Copy link
Copy Markdown
Contributor

do we start to merge those breaking changes?

@gportay
Copy link
Copy Markdown
Contributor Author

gportay commented Mar 24, 2025

I wonder if we should tag a 5.7 and wait for the 6.0.

@florentsfl
Copy link
Copy Markdown
Contributor

Ok like a 5.7 until podman support, and then we merge the breaking changes ?

@gportay
Copy link
Copy Markdown
Contributor Author

gportay commented Mar 24, 2025

Yes indeed. Also, I would like to add warnings for upcoming changes in 6.0.

I think about the deprecating the command cqfd run cmd args in favour of cqfd shell -c "cmd args" or cqfd exec cmd args. I have a change for that. I will make a PR, and see I you think it is a good change. I have in mind the PR about audoization that I think is probably a good CLI for cqfd.

Also, about the custom_img_name see #178.

@florentsfl
Copy link
Copy Markdown
Contributor

I just came across an old project containing semicolon comments in the .cqfdrc ^^

@gportay
Copy link
Copy Markdown
Contributor Author

gportay commented Apr 1, 2025

I just came across an old project containing semicolon comments in the .cqfdrc ^^

Switch them to shell comments :)

@gportay
Copy link
Copy Markdown
Contributor Author

gportay commented Apr 4, 2025

Also, I think we can introduce an environment such as CQFD_COMPAT variable, but honestly, I do not think it worse to maintain such things forever.

+ if [ "${CQFD_COMPAT:-57}" -le 56 ]; then
  ini=( ${ini[*]//;*/} )                            # remove comments with ;
+ fi

@florentsfl
Copy link
Copy Markdown
Contributor

or would it be possible to detect if the semicolon is the first char of a line and put a warning to tell to switch to # comments ?

@florentsfl florentsfl added this to the v6.0 milestone Jun 25, 2025
@gportay gportay force-pushed the stops-handling-comment branch 2 times, most recently from a391708 to 08e5447 Compare June 26, 2025 20:36
The .ini files uses the semicolon for comments. Also, that exact same
character is one of the shell control operator for list of pipelines.

The hacky parser is not smart enough to make the distinction if a
semicolon is an .ini comment or if it is a shell control operator.

It considers every semicolon character as a .ini comment, and it removes
the remaining characters till the EOL (including the semicolon), even if
they are part of a list of pipeline.

As a consequence, it cuts off the list of pipelines if the command=
values contain a semicolon character, and it results in running the
left-hand only.

This stops to handle the semicolon comments to avoid amputating the list
of pipelines controled by semicolons that are set in values of the
command= properties.

Note: The parser converts the .ini file to bash; as a consequence, the
shell comments do not affect the parsing, as long as the hashmark does
not break the syntax **AND** if it not put at the end of the section
line.
@gportay gportay force-pushed the stops-handling-comment branch from 08e5447 to 030f2c7 Compare July 8, 2025 20:40
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