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

"Ext" replacement (with expand) breaks on filenames that contain a dot #1022

Closed
wants to merge 1 commit into from
Closed

Conversation

dmp42
Copy link

@dmp42 dmp42 commented Dec 30, 2013

Have the following files:
$ ls bug/ a.b.js a.js b.js

Now:

    grunt.registerMultiTask('test', 'whatever', function() {
      this.files.forEach(function(f){
        console.warn(f.src, f.dest);
      });
    });

    grunt.config('test.build1', {
      files: [{
        expand: true,
        cwd: 'bug',
        src: ['**/*.js'],
        filter: 'isFile',
        dest: 'bar'
      }]
    });

    grunt.config('test.build2', {
      files: [{
        expand: true,
        cwd: 'bug',
        src: ['**/*.js'],
        filter: 'isFile',
        dest: 'bar',
        ext: 'whatever'
      }]
    });

grunt test.build1

outputs the expected:

[ 'bug/a.b.js' ] 'bar/a.b.js' [ 'bug/a.js' ] 'bar/a.js' [ 'bug/b.js' ] 'bar/b.js'

while
grunt test.build2

outputs:
[ 'bug/a.b.js', 'bug/a.js' ] 'bar/awhatever' [ 'bug/b.js' ] 'bar/bwhatever'

... which will result in concatenating a.js and a.b.js into awhatever (which IMHO is not expected).

Proposed patch fixes this.

@nschonni
Copy link

Rejected previously in #912 and a bunch of others.

@dmp42
Copy link
Author

dmp42 commented Dec 30, 2013

I get the "preserve backward compat" reasoning - though that could be landed on a non retro compatible branch.

So I just read #912 (comment) and #625 (comment) and I'll have to disagree about the whole "this is not a bug" stance - that really should be called "replaceEverythingAfterTheFirstDot" instead of "ext", if that behavior is really needed.

Really, nowhere is "else.txt" the "extension" of "something.else.txt" :(

The sheer number of people commenting +1 on these issues should be an indication that at the very least this is highly counter-intuitive...

Sorry for not finding these by myself, and thanks for your answer.

@dmp42 dmp42 closed this Dec 30, 2013
@nschonni
Copy link

No problem, I agree with you by the way. I use the rename approach and it's annoying 😉

@shama
Copy link
Member

shama commented Dec 30, 2013

I think it should be configurable both ways. At the time this decision needed to be made, Grunt was more commonly working with jquery.min.js names, so the decision makes sense to me. Also remember, multiple dot file extensions are probably more common then you think, suchas.tar.gz. So I wouldn't say "nowhere". ;)

A suggested API might look like:

task: {
  expand: true,
  cwd: 'lib/',
  src: '*.js.coffee',
  dest: 'dist',
  ext: { last: '.js' }
}

or something.

@tkellen
Copy link
Member

tkellen commented Dec 30, 2013

This is supported in node-globule, via extDot, which will eventually replace the current system in a future release of Grunt.

@dmp42
Copy link
Author

dmp42 commented Dec 31, 2013

Also remember, multiple dot file extensions are probably more common then you think, suchas.tar.gz. So I wouldn't say "nowhere". ;)

No, they are not :)

Call me an old fart :) but I don't think this makes sense...

  • such.as.tar.gz file extension is gz, not as.tar.gz.
  • suchas.thing.min.js file extension is js, not thing.min.js.

and none of these necessarily denote mime types, if that was what you had in mind with the tar.gz example.

If we want to special case the .min.js case, fair enough (as this seems to be the only reason to justify this), that should be simple, and made explicit in the used regexp:

/(\.(?:[^\/.]*|min[.]js))?$/

... if we really insist on calling that the "file extension"

Or, maybe we just need to change the documentation and call that stuff the "part of the file URI that follows the first dot", and/or change the name of the property.

But unless I'm really confused, there is not a single parsing library on earth that would give "as.thing.min.js" for a "file extension".

Just googling for the first hits:

or...

Yet again, I'll not argue about the behavior (of something I wouldn't use...) - but giving it the name of something else really doesn't make any sense and is clearly misleading people to unexpected results (yes, it took me some time to understand why my uglified files were not working...).

2 cents.

Best regards.

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

4 participants