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

Append a newline to the buffer if not present #32

Merged
merged 1 commit into from
Apr 12, 2020

Conversation

ashokadewit
Copy link

When a file was loaded that did not end with a newline a new variable
would be written on the same line as the last variable.

When a file was loaded that did not end with a newline a new variable
would be written on the same line as the last variable.
@JackieDo JackieDo merged commit 189ec00 into JackieDo:master Apr 12, 2020
@sergejostir
Copy link
Contributor

JackieDo added a commit that referenced this pull request Apr 12, 2020
@JackieDo
Copy link
Owner

JackieDo commented Apr 14, 2020

This will not work properly on Windows.
Fix: sergiz@b5685b9

@sergiz : I think the following is the best solution:

...
use Illuminate\Support\Str;
...

    /**
     * Set buffer with content
     *
     * @param  string $content
     *
     * @return DotenvWriter
     */
    public function setBuffer($content)
    {
        if (!empty($content) && !Str::endsWith($content, PHP_EOL)) {
            $content .= PHP_EOL;
        }

        $this->buffer = $content;

        return $this;
    }

How do you think?

@sergejostir
Copy link
Contributor

What's wrong with the current solution that you implemented?

$this->buffer = trim($content) . PHP_EOL;

@JackieDo
Copy link
Owner

What's wrong with the current solution that you implemented?

$this->buffer = trim($content) . PHP_EOL;

@sergiz : This solution I have implemented before your solution. But now realize that your solution is more complete.

@JackieDo
Copy link
Owner

@sergiz : However, I would like to change your solution a bit to prevent adding new lines when the .env file is empty.

@sergejostir
Copy link
Contributor

sergejostir commented Apr 14, 2020

I see. Well, the whole point here is to not use PHP_EOL constant, but '\n' when doing the check. This is why the initial PR is broken.

On windows PHP_EOL will be \r\n, so the check will fail, if the $content ends with just '\n'.

That's why I also kinda like your solution with trim because it will get rid of practically all useless characters. However I would propose using rtrim since someone might like to have an empty line at the beginning.

        if (!empty($content)) {
            $content = rtrim($content) . PHP_EOL;
        }

@JackieDo
Copy link
Owner

JackieDo commented Apr 14, 2020

@sergiz : Now we have two request:

  • Request 1: Prevent adding lines if the .env file is empty
  • Request 2: Use PHP_EOL instead of "\n"

What request do you agree with? (none, 1, 2 or both of all)

@sergejostir
Copy link
Contributor

sergejostir commented Apr 14, 2020

What do you mean?

        if (!empty($content)) {
            $content = rtrim($content) . PHP_EOL;
        }

Fits all. It doesn't add new line when the file is empty, and it makes sure that the content ends with a new line on all platforms.

@JackieDo
Copy link
Owner

What do you mean?

        if (!empty($content)) {
            $content = rtrim($content) . PHP_EOL;
        }

Fits all. It doesn't add new line when the file is empty, and it makes sure that the content ends with a new line on all platforms.

@sergiz : Yeah, this is the best solution.

@JackieDo
Copy link
Owner

@sergiz : This issue has been resolved. See f54431c.

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.

3 participants