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

dedents when it shouldn't #3

Open
brandonkal opened this issue Jan 10, 2020 · 6 comments · May be fixed by #4
Open

dedents when it shouldn't #3

brandonkal opened this issue Jan 10, 2020 · 6 comments · May be fixed by #4

Comments

@brandonkal
Copy link

brandonkal commented Jan 10, 2020

dedent`
one:
   two: 3
`

becomes

one:
two: 3
@MartinKolarik
Copy link
Owner

Looking at the code, I see that lines that have no indentation are ignored. It is not explicitly mentioned in the readme but I guess it makes sense, otherwise dedent would do nothing in those cases and there would be no point in using it.

@brandonkal brandonkal linked a pull request Jan 10, 2020 that will close this issue
@brandonkal
Copy link
Author

brandonkal commented Jan 10, 2020

I've created a fix and PR. I'm using this internal to a yaml template tag function. Naturally YAML is very indent focused but the function doesn't know ahead of time if a dedent needs to occur before parsing the string to a JavaScript object.

@KernelDeimos
Copy link

Looking at the code, I see that lines that have no indentation are ignored. It is not explicitly mentioned in the readme but I guess it makes sense, otherwise dedent would do nothing in those cases and there would be no point in using it.

Should dedent really be useful in this situation? I think consistent behaviour here has more advantages. Here are a few:

  • dedent becomes an idempotent operation if this quirk is removed
  • When dedent is used for the purpose of making code more readable (I would speculate this is the most common use case), indentation is lost when a string happens to start from all the way at the left
  • dedent-js could provide a flag to explicitly remove indentation up to a specified depth, if that's what's desired, covering more use cases overall while making behaviour more consistent

@KernelDeimos
Copy link

KernelDeimos commented Mar 1, 2021

Also Github reports that there are over 18 thousand repositories using this package. It is irresponsible to leave this awful quirk in the code.

@dvirtz
Copy link

dvirtz commented Aug 11, 2021

A possible workaround is to add a space at the begging of each line

dedent(text.replace(/(^[^\n]|\n)/g, '$1 '))

@KevinGhadyani-minted
Copy link

I think the issue here is you might be using dedent-js on something where you don't know the final tabbing. Because of that, it makes sense for dedent to do nothing. I'm also for this change.

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 a pull request may close this issue.

5 participants