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

Ability to set the destination filename #1

Closed
zkochan opened this issue Jan 4, 2017 · 6 comments
Closed

Ability to set the destination filename #1

zkochan opened this issue Jan 4, 2017 · 6 comments

Comments

@zkochan
Copy link

zkochan commented Jan 4, 2017

I'd like to use this package in pnpm, but I need the possibility to set the symlink filename. Currently it takes the source filename and that is not good for all of my use cases.

So instead of this:

lnk('assets/favicon.ico', 'dist')

I'd need something like

lnk('assets/favicon.ico', 'dist/newfavicon.ico')

I don't know what would be the best way to add this feature. Maybe changing the default behavior? That would be closer to the fs.symlink API

@schnittstabil
Copy link
Owner

schnittstabil commented Jan 4, 2017

Sadly, 'dist/newfavicon.ico' would be ambiguous, it could denote a directory or a file path.

I would opt for a rename option (stringfunction):

// string
lnk('assets/favicon.ico', 'dist', {rename: 'subpath/newfavicon.ico'});
// => 'dist/subpath/newfavicon.ico'

// function
lnk(globby('assets/fav*.ico'), 'dist', {
    rename: ({dirname, basename}) => `${dirname}/new-${basename}`
    // or
    rename: ({path}) => 
});

Implementing the function feature would take some time, but the string feature would be simple. @zkochan Would this work for you?

@zkochan
Copy link
Author

zkochan commented Jan 4, 2017

Actually, for now the existing API worked for me:
https://github.com/rstacruz/pnpm/blob/pure-store/src/install/symlinkToModules.ts#L46

For now I will use both this package and link-dir. It has symlink overwriting, which we also need in pnpm.

Sadly, 'dist/newfavicon.ico' would be ambiguous, it could denote a directory or a file path.

Unless there's a breaking change and the function always expects the full path, right? That would also mean that the first argument can't be an array of targets anymore.

I like the first solution with the string. Could it be a 3rd parameter maybe?

lnk('assets/favicon.ico', 'dist', 'subpath/newfavicon.ico');
// => 'dist/subpath/newfavicon.ico'

@schnittstabil
Copy link
Owner

That would also mean that the first argument can't be an array of targets anymore.

lnk allows arrays because this simplifies many use-cases, especially in conjunction with the parents option.

Could it be a 3rd parameter maybe?

IMO, that would be too cumbersome; and moreover, it could be confused with the cwd option as well.


For now I will use both this package and link-dir. It has symlink overwriting, which we also need in pnpm.

It is not clear to me, how lnk and link-dir differ. lnk offers a force option for overwriting too.

From the source you've linked:

return linkDir(absolutePath, dest)

At a first glance this could also be done with lnk by:

return lnk(absolutePath, dest, {type: 'junction'})

@zkochan
Copy link
Author

zkochan commented Jan 4, 2017

ok, seems like opts.rename is the best solution in this situation.

It is not clear to me, how lnk and link-dir differ. lnk offers a force option for overwriting too.

Thanks, I'll check the force option today, maybe that will do

@schnittstabil
Copy link
Owner

schnittstabil commented Jan 7, 2017

Landed with v1.1.0 – see opts.rename for details.

@zkochan I hope these work for you:

lnk('assets/favicon.ico', 'dist', {rename: 'newfavicon.ico'});

// or e.g. with globs
lnk(globby('assets/fav*.ico'), 'dist', {
    rename: ({base}) => `new${base}`
});

@zkochan
Copy link
Author

zkochan commented Jan 7, 2017

Thank you!

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

2 participants