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

latest slash path is been ignored #438

Closed
EthraZa opened this issue Sep 14, 2021 · 25 comments
Closed

latest slash path is been ignored #438

EthraZa opened this issue Sep 14, 2021 · 25 comments

Comments

@EthraZa
Copy link

EthraZa commented Sep 14, 2021

I started to get this problem in newer versions:
(TBH, I still don't undestand this error, but I guess it is related, so, I' m showing it here)

$ minify  --match '\.(css|html|js|json)$' -sarp 'www/dev/' -o 'www/hml/'
ERROR: stat www/dev//www/dev/autoindex: no such file or directory

$ ls -l www/dev/autoindex
lrwxrwxrwx ... www/dev/autoindex -> lib/autoindex/js/NGINdeX.io/

But then, testing, if I remove the link from dev folder, and test it with files and directories only, I found this:

$ minify  --match '\.(css|html|js|json)$' -sarp 'www/dev/' -o 'www/hml/'

$ ls -l www/hml
total 1
drwxr-xr-x ... dev

And that is wrong!
I wanted the dev folder content under hml folder and not dev folder under hml folder, because of that I used www/dev/ and not www/dev (notice the slash at the end).

It was working before, but the behavior changed somewhere in the way.

@tdewolff
Copy link
Owner

Yes, this behaviour has changed to reflect the results of the cp command better. That is, it should be more intuitive or in-line with Linux customs. You should change the input www/dev/ into www/dev/* so that the files inside www/dev will be moved into www/html. Please let me know if that works!

@tdewolff
Copy link
Owner

tdewolff commented Oct 4, 2021

Closing now, please reply / re-open if you have doubts or questions!

@tdewolff tdewolff closed this as completed Oct 4, 2021
@EthraZa
Copy link
Author

EthraZa commented Oct 4, 2021

Sorry for the delay.

/* does not work as well

sailor@spock:/tmp$ minify  --match '\.(css|html|js|json)$' -sarp 'www/dev/*' -o 'www/hml/'
ERROR: stat www/dev/*: no such file or directory

sailor@spock:/tmp$ ll www/dev/
total 116
drwxrwxr-x 3 sailor sailor    100 Out  4 15:05 ./
drwxrwxr-x 4 sailor sailor     80 Out  4 15:08 ../
-rw-r--r-- 1 sailor sailor   7098 Out  4 15:04 index.html
drwxrwxr-x 2 sailor sailor     80 Out  4 15:05 sub/
-rw-r--r-- 1 sailor sailor 106720 Out  4 15:05 sys.js
sailor@spock:/tmp$ ll www/hml/
total 0
drwxrwxr-x 2 sailor sailor 40 Out  4 15:06 ./
drwxrwxr-x 4 sailor sailor 80 Out  4 15:08 ../

@tdewolff
Copy link
Owner

tdewolff commented Oct 4, 2021

What shell are you using? www/dev/* should be expanded to the files in that directory by the shell (not by minify)

@EthraZa
Copy link
Author

EthraZa commented Oct 4, 2021

sailor@spock:/tmp$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04.7 LTS"

sailor@spock:/tmp$ echo $0
-bash

sailor@spock:/tmp$ bash --version
GNU bash, versão 4.3.48(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2013 Free Software Foundation, Inc.
Licença GPLv3+: GNU GPL versão 3 ou posterior <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

@tdewolff
Copy link
Owner

tdewolff commented Oct 4, 2021

Sorry, you need to remove the quotes around 'www/dev/*', because with the quotes it means that we want to minify the literal file www/dev/*, instead the * should be a wildcard.

@tdewolff
Copy link
Owner

tdewolff commented Oct 4, 2021

minify --match '\.(css|html|js|json)$' -sarp www/dev/* -o www/hml/

should work

@EthraZa
Copy link
Author

EthraZa commented Oct 4, 2021

Nope

sailor@spock:/tmp$ ll www/dev/
total 116
drwxrwxr-x 3 sailor sailor    100 Out  4 16:00 ./
drwxrwxr-x 4 sailor sailor     80 Out  4 16:02 ../
-rw-rw-r-- 1 sailor sailor   7098 Out  4 16:01 index.html
drwxrwxr-x 2 sailor sailor     60 Out  4 16:01 sub/
-rw-rw-r-- 1 sailor sailor 106720 Out  4 16:01 sys.js

$ minify  --match '\.(css|html|js|json)$' -sarp www/dev/* -o www/hml/
2021-10-04 16:01:43.955863984 -0300 -03 2021-10-04 16:01:20.871933588 -0300 -03
2021-10-04 16:02:16.779764968 -0300 -03 2021-10-04 16:01:43.955863984 -0300 -03
2021-10-04 16:02:16.779764968 -0300 -03 2021-10-04 16:01:36.875885335 -0300 -03

sailor@spock:/tmp$ ll www/hml/
total 0
drwxrwxr-x 4 sailor sailor 80 Out  4 16:02 ./
drwxrwxr-x 4 sailor sailor 80 Out  4 16:02 ../
drwxrwxr-x 2 sailor sailor 80 Out  4 16:02 dev/
drwxrwxr-x 2 sailor sailor 60 Out  4 16:02 sub/

It's working in the sence it is minifying, but it is creating the dev subfolder inside the hml and moving files there and, yet, the sub folder is not going inside dev, it's gooing to the hml root.

Edit: I believe the behavior with folders (sub) is the right one, not the behavior of files that is creating a new folder with the same name of the source.
Is my english making sence? :)

tdewolff added a commit that referenced this issue Oct 4, 2021
@tdewolff
Copy link
Owner

tdewolff commented Oct 4, 2021

Yes it's making complete sense! I've pushed out a fix that hopefully works well (there are many edge-cases), please let me know how it goes.

@vszakats
Copy link

vszakats commented Aug 25, 2023

I've made a little test while struggling to use minify to work in-place both with recurse and with single-files. Behaviour also changes between versions. The only workaround I found to work reliably across versions and use-cases, is to cd into the target directory and call minify . --output .. Everything else gave unexpected results in one situation or another. Test a3 makes minify create recursive output directories with subsequent runs with the same arguments.

Here's the self-contained test, with results for v2.12.8 (macOS, via the minify Homebrew tap) and v2.12.4 (Debian bookworm):

#!/bin/sh

# Test running minify with various input/output directory combinations with
# the intent to minify the input file in place.
# Test both `--recurse` and single-file invocations via the `find` command.
#
# Put this script inside a subdirectory of a new empty directory, e.g.
# <home>/my-test/script/minify-tests.sh
#
# `--output` trailing slash did not make any difference in these tests, though
# I added it to comply with the manual saying '(must have trailing slash)'.
#
# OK  : input was minified (=became a one-liner) in-place
# FAIL: input was not minified or the result was saved in a new location

cd "$(dirname "$0")" || exit 1

cat > index.html <<EOF
<!DOCTYPE html>
<html lang="en">

<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="referrer" content="no-referrer">


<link rel="canonical" href="https://example.com/">

<link rel="stylesheet" href="/assets/css/main.css">


<title>my page</title>
<body>
EOF

cat > site.webmanifest <<EOF
{
  "name": "",
  "icons": [
    { "src": "/assets/icon/touch-icon-192.png", "sizes": "192x192" },
    { "src": "/assets/icon/touch-icon-512.png", "sizes": "512x512" }
  ],
  "background_color": "#ffffff",
  "theme_color": "#ffffff",
  "display": "standalone"
}
EOF

# Run minify in a absolute directory that is not a subdirectory of the current directory
rm -rf ../a1 ../a2 ../a3 ../a4 ../a5

# v2.12.8 (macOS)  v2.12.4 (Debian)
# OK  /FAIL        OK  /OK
input="$(realpath ../a1)"; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}"  --recursive --output "${input}/../" --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# OK  /FAIL        FAIL/OK
input="$(realpath ../a2)"; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}/" --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# FAIL/FAIL        FAIL/OK
input="$(realpath ../a3)"; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}"  --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# repeat test:
# FAIL/FAIL        FAIL/FAIL  NOTE: recurses into the output directory ending up with ../a3/a3/a3/index.html
minify "${input}"  --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# FAIL/FAIL        FAIL/OK
input="$(realpath ../a4)"; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}"  --recursive --output ./ --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# OK  /OK          OK  /OK
input="$(realpath ../a5)"; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
(
  cd "${input}" || exit 1
  minify . --recursive --output ./ --verbose
  find . -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;
)

# Run minify in a subdirectory of the current directory
rm -rf r1 r2 r3 r4 r5

# OK  /OK          OK  /OK
input='r1'; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}"  --recursive --output "${input}/../" --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# OK  /OK          FAIL/OK
input='r2'; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}/" --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# FAIL/OK          FAIL/OK
input='r3'; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}"  --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# OK  /OK          OK  /OK
input='r4'; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
minify "${input}"  --recursive --output ./ --verbose
find "${input}" -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;

# OK  /OK          OK  /OK
input='r5'; mkdir -p "${input}"; cp index.html "${input}"; cp site.webmanifest "${input}"
(
  cd "${input}" || exit 1
  minify . --recursive --output ./ --verbose
  find . -name '*.webmanifest' -exec minify --type=json --output ./ --verbose {} \;
)

rm -rf index.html site.webmanifest

It's possible I've made some mistakes in registering results and even more possible that I had missed the right way to call minify. Case r5 and a5 are the method that worked for me.

@tdewolff
Copy link
Owner

Thank you for the extensive test, I will look into all cases.

@vszakats
Copy link

Thank you @tdewolff, much appreciated. Also thanks for making minify!

@tdewolff
Copy link
Owner

tdewolff commented Sep 9, 2023

I've pushed out a (possible) fix, including tests so this doesn't happen again. Please test for yourself as well, for me all your tests work as expected, including the a3/r3 recursive output.

I used to model for equivalent functionality as cp, but with the trailing slashes I try to follow rsync instead.

@vszakats
Copy link

Thank you very much for your updates. I'll make tests after next week.

@vszakats
Copy link

I retested with the latest Git version (sorry for the delay!). One improvement is that the target directories are no longer re-created in the current directory with tests a1, a2, a3. With a4 it's still created. On the other hand site.webmanifest is no longer minified for the r1, r2, r3, r4 cases, that seems to be a regression. a5/r5 continue to work as expected, but with other tests I cannot confirm the expected results.

Also with the latest version, the original temp source file site.webmanifest in the current directory was unexpectedly overwritten by test a1. I updated the script to recreated originals before each test (and also to avoid requiring realpath, plus added new test results):

#!/bin/sh

# Posted to https://github.com/tdewolff/minify/issues/438 v2

# Test running minify with various input/output directory combinations with
# the intent to minify the input file in place.
# Test both `--recurse` and single-file invocations via the `find` command.
#
# Put this script inside a subdirectory of a new empty directory, e.g.
# <home>/my-test/script/minify-tests.sh
#
# `--output` trailing slash did not make any difference in these tests, though
# I added it to comply with the manual saying '(must have trailing slash)'.
#
# OK  : input was minified (=became a one-liner) in-place
# FAIL: input was not minified or the result was saved in a new location

cd "$(dirname "$0")" || exit 1

initfiles() {
cat > index.html <<EOF
<!DOCTYPE html>
<html lang="en">

<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="referrer" content="no-referrer">


<link rel="canonical" href="https://example.com/">

<link rel="stylesheet" href="/assets/css/main.css">


<title>my page</title>
<body>
EOF

cat > site.webmanifest <<EOF
{
  "name": "",
  "icons": [
    { "src": "/assets/icon/touch-icon-192.png", "sizes": "192x192" },
    { "src": "/assets/icon/touch-icon-512.png", "sizes": "512x512" }
  ],
  "background_color": "#ffffff",
  "theme_color": "#ffffff",
  "display": "standalone"
}
EOF
}

minify=~/.go/bin/minify

# Run minify in a absolute directory that is not a subdirectory of the current directory
rm -rf ../a1 ../a2 ../a3 ../a4 ../a5

# v2.12.8 (macOS)  v2.12.4 (Debian)  dfaa2fcde
# OK  /FAIL        OK  /OK           OK  /FAIL
input="$(pwd)/../a1"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive --output "${input}/../" --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# OK  /FAIL        FAIL/OK           OK  /FAIL
input="$(pwd)/../a2"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}/" --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# FAIL/FAIL        FAIL/OK           FAIL/FAIL
input="$(pwd)/../a3"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# repeat test:
# FAIL/FAIL        FAIL/FAIL         FAIL/FAIL  NOTE: recurses into the output directory ending up with ../a3/a3/a3/index.html
"${minify}" "${input}"  --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# FAIL/FAIL        FAIL/OK           FAIL/FAIL
input="$(pwd)/../a4"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive --output ./ --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# OK  /OK          OK  /OK           OK  /OK
input="$(pwd)/../a5"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
(
  cd "${input}" || exit 1
  "${minify}" . --recursive --output ./ --verbose
  find . -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;
)

# Run minify in a subdirectory of the current directory
rm -rf r1 r2 r3 r4 r5

# OK  /OK          OK  /OK           OK  /FAIL
input='r1'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive --output "${input}/../" --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# OK  /OK          FAIL/OK           OK  /FAIL
input='r2'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}/" --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# FAIL/OK (rec.)   FAIL/OK           OK  /FAIL (rec.)
input='r3'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive --output "${input}/" --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# OK  /OK          OK  /OK           OK  /FAIL
input='r4'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive --output ./ --verbose
find "${input}" -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;

# OK  /OK          OK  /OK           OK  /OK
input='r5'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
(
  cd "${input}" || exit 1
  "${minify}" . --recursive --output ./ --verbose
  find . -name '*.webmanifest' -exec "${minify}" --type=json --output ./ --verbose {} \;
)

rm -rf index.html site.webmanifest

@tdewolff
Copy link
Owner

tdewolff commented Sep 28, 2023

Thanks again for the in-depth analysis. What does OK /FAIL mean, or what is the difference between the first and second column for each version? When running the script, it minifies all files but there are no errors thrown (i.e. all OK?).

The a3 recursive test is expected to work like that by the way, this is what happens with cp and with rsync as well. Just in case, the expected result for each test (absolute vs relative should be the same):

  1. Minify the directory r1 into r1/../, this overwrites the files
  2. Minify contents of r2/ (because of the trailing slash) into r2/, this overwrites the files
  3. Minify the directory r3 into r3/, that is it writes to r3/r3/.
  4. Minify the directory r4 into ./, this overwrites the files
  5. Go into r5 and minify . into ./, this overwrites the files

Problem:

  • a1,a2,a3,a4 and r1,r2,r3,r4 overwrite the original site.webmanifest and not to the output destination

Otherwise, I see that the a4 ends up in the same directory as the script, but this is expected as the output is ./ (relative to the script). The rest seems OK to me, happy to hear your doubts. I will work on the site.manifest problem!

@vszakats
Copy link

vszakats commented Sep 28, 2023

Sorry for the confusion! OK/FAIL means that the html input was correctly minified, but the .webmanifest was not. It's the results for the two files the script is testing, respectively.

What I'm looking for is a combination that works alike a5 and r5, but without having to change the current directory.

Indeed, the remaining issue is site.webmanifest, many thanks for looking into this!

@tdewolff
Copy link
Owner

Looking into the site.manifest issue, it's actually what I'd expect. find returns all matching files but doesn't change the current working directory (e.g. find . -name '*.webmanifest' -exec pwd \; prints the same directory for all matches). This means that --output ./ means "minify into the current directory". And this is exactly what happens.

Some options:

  • Use find . -name '*.sitemanifest' -exec minify {} -o {} -v \;
  • Use minify --match='*.sitemanifest' --type=json -o $output $input
  • We could add a extension/mime type mapping functionality to minify, so you can specify `--map-ext='.sitemanifest=json,.someother=xml' or something similar
  • We could add .sitemanifest as an extension for JSON by default

@vszakats
Copy link

vszakats commented Sep 28, 2023

We could add .sitemanifest as an extension for JSON by default

This was indeed the reason for the find exception: to handle this
specific file type as JSON. If minify could recognize it as such,
that'd make this much simpler.

The filename extension is .webmanifest. (as in site.webmanifest).

Even though the spec
only recommends this specific extension, there seems to be an agreement, that
site.webmanifest is the best choice, because it needs to be served with its
special MIME-type anyway.

Also thanks for your suggestions!

I started testing with minify --match='*.sitemanifest' --type=json -o $output $input,
errored (ERROR: error parsing regexp: missing argument to repetition operator: '*'),
but this one works as expected:
minify --recurse --match='.*\.sitemanifest' --type=json -o $output $input

This halved the amount of test cases.

One remaining issue is that the Debian-supplied older v2.12.4 has a2, r2
broken, but this is the method I'm aiming at anyway.

Also a1/r1 works with all versions, but the output parameter (upper directory)
seems to be fragile to generate.

Updated test script:

#!/bin/sh

# Posted to https://github.com/tdewolff/minify/issues/438 v3

# Test running minify with various input/output directory combinations with
# the intent to minify the input file in place.
# Test both `--recurse` and single-file invocations via the `find` command.
#
# Put this script inside a subdirectory of a new empty directory, e.g.
# <home>/my-test/script/minify-tests.sh
#
# `--output` trailing slash did not make any difference in these tests, though
# I added it to comply with the manual saying '(must have trailing slash)'.
#
# OK  : input was minified (=became a one-liner) in-place
# FAIL: input was not minified or the result was saved in a new location

cd "$(dirname "$0")" || exit 1

initfiles() {
cat > index.html <<EOF
<!DOCTYPE html>
<html lang="en">

<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="referrer" content="no-referrer">


<link rel="canonical" href="https://example.com/">

<link rel="stylesheet" href="/assets/css/main.css">


<title>my page</title>
<body>
EOF

cat > site.webmanifest <<EOF
{
  "name": "",
  "icons": [
    { "src": "/assets/icon/touch-icon-192.png", "sizes": "192x192" },
    { "src": "/assets/icon/touch-icon-512.png", "sizes": "512x512" }
  ],
  "background_color": "#ffffff",
  "theme_color": "#ffffff",
  "display": "standalone"
}
EOF
}

minify=~/.go/bin/minify

# Run minify in a absolute directory that is not a subdirectory of the current directory
rm -rf ../a1 ../a2 ../a3 ../a4 ../a5

# v2.12.8 (macOS)  v2.12.4 (Debian)  dfaa2fcde/v2.12.8-brew
# OK               OK                OK
input="$(pwd)/../a1"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive                                       --output "${input}/../" --verbose
"${minify}" "${input}"  --recursive --match='.*\.webmanifest' --type=json --output "${input}/../" --verbose

# OK               FAIL (rec)        OK
input="$(pwd)/../a2"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}/" --recursive                                       --output "${input}/" --verbose
"${minify}" "${input}/" --recursive --match='.*\.webmanifest' --type=json --output "${input}/" --verbose

# FAIL             FAIL              FAIL
input="$(pwd)/../a3"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive                                       --output "${input}/" --verbose
"${minify}" "${input}"  --recursive --match='.*\.webmanifest' --type=json --output "${input}/" --verbose

# repeat test:
# FAIL (rec.)      FAIL (rec.)       FAIL (rec.)
"${minify}" "${input}"  --recursive                                       --output "${input}/" --verbose
"${minify}" "${input}"  --recursive --match='.*\.webmanifest' --type=json --output "${input}/" --verbose

# FAIL             FAIL              FAIL
input="$(pwd)/../a4"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive                                       --output ./ --verbose
"${minify}" "${input}"  --recursive --match='.*\.webmanifest' --type=json --output ./ --verbose

# OK               OK                OK
input="$(pwd)/../a5"; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
(
  cd "${input}" || exit 1
  "${minify}" . --recursive                                       --output ./ --verbose
  "${minify}" . --recursive --match='.*\.webmanifest' --type=json --output ./ --verbose
)

# Run minify in a subdirectory of the current directory
rm -rf r1 r2 r3 r4 r5

# OK               OK                OK
input='r1'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive                                       --output "${input}/../" --verbose
"${minify}" "${input}"  --recursive --match='.*\.webmanifest' --type=json --output "${input}/../" --verbose

# OK               FAIL              OK
input='r2'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}/" --recursive                                       --output "${input}/" --verbose
"${minify}" "${input}/" --recursive --match='.*\.webmanifest' --type=json --output "${input}/" --verbose

# FAIL (rec.)      FAIL (rec.)       FAIL (rec.)
input='r3'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive                                       --output "${input}/" --verbose
"${minify}" "${input}"  --recursive --match='.*\.webmanifest' --type=json --output "${input}/" --verbose

# OK               OK                OK
input='r4'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
"${minify}" "${input}"  --recursive                                       --output ./ --verbose
"${minify}" "${input}"  --recursive --match='.*\.webmanifest' --type=json --output ./ --verbose

# OK               OK                OK
input='r5'; mkdir -p "${input}"; initfiles; cp index.html "${input}"; cp site.webmanifest "${input}"
(
  cd "${input}" || exit 1
  "${minify}" . --recursive                                       --output ./ --verbose
  "${minify}" . --recursive --match='.*\.webmanifest' --type=json --output ./ --verbose
)

rm -rf index.html site.webmanifest

@vszakats
Copy link

vszakats commented Sep 28, 2023

I tend to think an option to do every file operation in place would be
the most natural solution to this.

At the moment it's almost unpredictable where something will be
written to and what will be accidentally overwritten, recursively created
or just copied into an unexpected location in the process. A single ending
slash may result in the output going into a different destination.
Behaviour also changes subtly between minor versions. (E.g. I just found
a case ("r6") that I missed from the test script, which would work correctly
on Debian bookworm (minify './_site/' --output './' --recursive, and what
had overwritten my development folder when using the newer, Homebrew
version. This one seems to work fine (with relative dirs) in both envs:
minify './_site' --output './' --recursive)

My use-case is a static website generator where the output is already
in a distinct directory. Minifcation should happen in there without the
chance of touching anything outside this directory. The rsync or cp
analogy doesn't play well with this use-case, since there is no intention
to copy content.

Something like this would work without any doubts:

$ minify . --inplace --recursive [...]

(show error if an --output option is also supplied.)

Perhaps also a --dry-run option might help to understand what would
happen without actually making it happen.

Together with recognizing .webmanifest files, this would transform
into an easy use-case.

On more minor bit: Docs says that the output directory must end with
a slash. In my tests it didn't make a difference. If this is no longer
necessary, it'd be nice to delete it from the help output. If it is always
necessary, minify could add automatically if missing.

A v4 of the test script with four new combinations: a6/r6/a7/r7.
minify-tests-v4.sh.zip

BTW, for now I settled with a1 and r4. This works with bookworm and
Homebrew and the latest master. Plan to go to a2 once Debian ships
with a newer version.

tdewolff added a commit that referenced this issue Sep 29, 2023
@tdewolff
Copy link
Owner

tdewolff commented Sep 29, 2023

I can't change the past, so I'm only concerned with getting the latest version bug-free. This issue has helped a lot in making sure the tool works correctly, but previous versions will naturally behave differently. It's not my intention to make all version behave the same, instead I want to make it work as expected. If you really need a solution that works for all versions, it might be better to use common bash tools, such as: find . -name=*.html -exec minify -v -o {} {} \;.

The in-place option that you refer to is easily achieved by using the same input and output directories: minify -v -r -o dir/ dir/. The default behaviour is to output to stdout (common use-case for command line piping), and otherwise the usual approach is to minify to another directory so as to not overwrite files by default. This is really similar to copying files (cp or rsync) which is the reason for the analogy. If you really want to minify in-place, you should say so explicitly such as the command above.

Adding --dry-run could be an option, though not sure if it is really that useful. Otherwise, adding more common file formats such as .webmanifest and others could be useful, or providing an extension => file type mapping.

There really is a difference with -o path and -o path/. In the former case, if you specify multiple input files and path exists and is a file, it will force you to set --bundle and it will output a single file. If the file doesn't exist it will create a directory path. If you only minify a single file, and path exists and is a directory, it will write into that directory. Otherwise, it will create a file path. Adding the final slash removes ambiguity, it will always write to a directory.

Regarding --match, yes sorry that should've been a regular expressions. Perhaps we should support both regexes and globs.

Regarding your new tests, I'm not sure what your confusion is as both work as expected with the latest version. What did you expect?

  • r6/a6: minify the files in r6 and output them to the current directory of the script, working as expected
  • r7/a7: minify the files in r7 into the directory above, working as expected

I've added a commit, what remains is 1) adding more common web file format extensions, 2) support for --ext-map

Thanks for all the time and in-depth analysis!!

tdewolff added a commit that referenced this issue Sep 29, 2023
…or index.*, otherwise use regexp as before; the glob pattern will match the final part of the path, see #438
@tdewolff
Copy link
Owner

Support for wildcards in --match is now added.

@vszakats
Copy link

vszakats commented Sep 29, 2023

No worries, me also thanks for your fixes and input, and taking your time to discuss this.

I hope it will get better, especially once Debian upgrades minify to a newer version. Then I can move (back) to the clean minify -v -r -o dir/ dir/ (a2/r2), and also bump --preserve to --preserve=all. And maybe even make good use of .webmanifest support or --ext-map=, if that makes it into that version.

[ If you ever consider an --in-place option, it might also enable --preserve-all by default. This could save some learning curve for users needing this. It'd also solve backward compatibility woes, as this would only work in newer versions. ]

tdewolff added a commit that referenced this issue Oct 6, 2023
@tdewolff
Copy link
Owner

tdewolff commented Oct 6, 2023

I've added support for the --ext option! You can use it as:

$ minify -r -o out/ --ext.scss=text/css --ext.xjs=js src/

or

$ minify -r -o out/ --ext {scss:text/css xjs:js} src/

@vszakats
Copy link

vszakats commented Oct 6, 2023

@tdewolff: Looks very cool!

Just tested it, and this worked for me for the .webmanifest files:

$ minify -r -o out/ --ext.webmanifest=json src/

Missing your commit ref 39b0c09 above, I just realized that minify now recognizes .webmanifest automatically, so even that is not necessary now. And simply $ minify -r -o out/ src/ tested ok.

Thank you so much for these!

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

No branches or pull requests

3 participants