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

[New] add `--indent` CLI arg; add tabs support #126

Open
wants to merge 1 commit into
base: master
from

Conversation

@ljharb
Copy link

ljharb commented Jan 8, 2020

Fixes #121

Happy to add tests and usage examples if this is the right direction to be going.

(Separately, i think thinks would be a lot cleaner if you used something like yargs as an arg parser, but that's out of scope for this PR)

Fixes #121
@antonmedv

This comment has been minimized.

Copy link
Owner

antonmedv commented Jan 8, 2020

I don’t like adding more flags to fx. Can we use .fxrc cars for this setting?
Also I try to use as little bpm packages as possible. This allows to faster startup time.

@ljharb

This comment has been minimized.

Copy link
Author

ljharb commented Jan 8, 2020

I'm not sure why using npm packages would meaningfully increase startup time, but sure, that's up to you.

As for flags - .fxrc is not an option for me because i want to use it as a systemwide utility, in any project. I could use an environment variable, however, if that would be preferable?

@antonmedv

This comment has been minimized.

Copy link
Owner

antonmedv commented Jan 9, 2020

I'm not sure why using npm packages would meaningfully increase startup time, but sure, that's up to you

It will. How you suppose it can be done without extra costs? For example adding yargs adds around 50 ms delay.

And I trying to keep fx under 100 ms startup time.

I could use an environment variable, however, if that would be preferable?

Yes, env is better. Also can you support other params too?

@ljharb

This comment has been minimized.

Copy link
Author

ljharb commented Jan 9, 2020

Sure, what'd you have in mind? Most of the config things are functions (impossible to support via env vars), or shell colors (awkward to support via env vars), so this seems like the only one.

@antonmedv antonmedv force-pushed the antonmedv:master branch from 7e93e7b to e7619ae Jan 9, 2020
@antonmedv

This comment has been minimized.

Copy link
Owner

antonmedv commented Jan 9, 2020

Hmmm, yes. You are right. Let's find some solutions. Maybe completely refactor to env instead of function?

@ljharb

This comment has been minimized.

Copy link
Author

ljharb commented Jan 9, 2020

That's certainly an approach - but it might be better to wait for a use case to present itself before creating churn, and i'm sure .fxrc makes a lot of sense for many use cases. At the moment the only new use case i'm aware of is for indentation :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.