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

Should only trim one newline at start and end so that you can add custom newlines #41

Open
dword-design opened this issue Jul 2, 2023 · 11 comments
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request

Comments

@dword-design
Copy link

Looks like the module removes all newlines from start and end, but I think it makes more sense to only remove one newline at start and end so that I can add newlines without adding \n.

// Adds a newline at the end. Currently the newline would be deleted.
dedent`
  foo
  bar

`
@JoshuaKGoldberg
Copy link
Collaborator

Blurgh, and then I missed you filing this issue - sorry about that @dword-design!

I think this is pretty reasonable. Dedenting is a separate action from trimming. I think it'd be reasonable to stop trimming any newlines at all. Now that we have a place for options (#65), it might be good to add an option like trim that's off by default. What do you think?

cc @G-Rath and @Haroenv since you've been active here recently too, thoughts?

@JoshuaKGoldberg JoshuaKGoldberg added the status: in discussion Not yet ready for implementation or a pull request label Jul 30, 2023
@G-Rath
Copy link
Contributor

G-Rath commented Jul 30, 2023

I think dedent should definitely trim at least the first and last newline by default, otherwise it defeats the point right?

I'm not sure if you need a dedicated trim function when you can just call .trim() on the string (if you go with just trimming off the one first/last newline)

@Haroenv
Copy link
Collaborator

Haroenv commented Jul 31, 2023

I think trimming makes sense by default, but maybe when there are multiple it should be preserved? Not sure what the actual use case is that does need a non-indented string, but also has newlines that have a use case?

@dword-design
Copy link
Author

dword-design commented Jul 31, 2023

Trimming one newline at begin and end and keep the others is basically what I have proposed in this PR and would be the expected behavior from my side.

@mariomui
Copy link

mariomui commented Nov 30, 2023

const markdown2 = `---
list: me
bird: two
---
poo
`;
const markdown = dedent`
	---
	list: me
	bird: two
	---
	poo\n
	`;

currently i have to do this because of the trimming. maybe have an option to turn off trimming?

@CxRes
Copy link

CxRes commented Jun 27, 2024

Could we bring @dword-design PR back under a flag, please? This way it is not a breaking change and those who want can opt into this behaviour.

@dword-design
Copy link
Author

I myself don't need it anymore since I now do it with \n, but I'm happy to help.

@CxRes
Copy link

CxRes commented Jun 29, 2024

@dword-design Call me peculiar, but doing it with \n (which I do too) looks just ugly. Also, string.dedent proposal will work the way you are suggesting. A flag seems like a reasonable compromise without breaking backwards compatibility!

@dword-design
Copy link
Author

@CxRes I mainly used it for a trailing newline in file-based tests and I finally decided that

dedent`
  # bla

  blub\n
`

is more compact than

dedent`
  # bla

  blub

`

But this is just my particular use case.

@CxRes
Copy link

CxRes commented Jun 29, 2024

\n\n @dword-design Sure, this is small potatos! I just find this funny that we should start writing like this.\n :)

@JoshuaKGoldberg
Copy link
Collaborator

Coming back to this: I still think it's a reasonable request. But, the dedent library is very old and a lot of apps depend on its behavior. I think we should keep the current behavior as the default and only put new behaviors behind a new option.

Specifically, I think there are three use cases one could want for a trim option:

  • 'all' (current): trim everything
  • 'none': disable trimming altogether
  • 'one': only trim one newline on each side

Accepting PRs for a new trim option to change the trimming behavior. 👍

@JoshuaKGoldberg JoshuaKGoldberg added type: feature New enhancement or request status: accepting prs Please, send a pull request to resolve this! and removed status: in discussion Not yet ready for implementation or a pull request labels Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request
Projects
None yet
Development

No branches or pull requests

6 participants