Webbased editor changes all end-of-line markers.

Created by: nielsbasjes

Summary: Editing a file using the webbased editor changes the line endings to what the browser uses.

Steps to reproduce: I create a text file (i.e. some sourcecode using vim) under Linux and commit and push it to a git repo under gitlab. Then I see a small typo and edit the file using the webbased editor from my Chrome under Windows 7. After I commit the changed file it turns out that ALL lines of the file have been changed because the "UNIX" end-of-line has been replaced with the "Windows" end-of-line.

Expected behavior: The line endings of the untouched lines remain unchanged. I'm unsure what the behaviour of changed and new lines should be.

Observed behavior: ALL line endings have changed into "Windows style" when editing under windows.

Relevant logs and/or screen shots: When I look in the change set of a commit did as a test (I changed nothing, I just did a commit from the webeditor) on a file with 7 lines: "Showing 0 changed files" and "This commit has 7 additions and 7 deletions"

Output of checks Results of GitLab Application Check (sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production); we will only investigate if the tests are passing

All tests are passing. Only a single unimportant notice about the init.d script not being up to date.

Version of GitLab you are running; 5.2.0 02a7b127 Describe your setup (use relevant parts from sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production) CentOS 6.4, Git 1.8.3.1

Possible fixes: Have a setting next to the "Save" button of the webeditor that specifies the desired line endings (Unix/Dos/Mac/...), then at save of the webeditor forces this on the entire file. The setting should default to the line ending type that occurs most in the given file.

Created by: Razer6

Seems to be a duplicate of #3677 (closed). But your one has the better describtion. The oder issue more describes the result not the root cause. /cc @senny Please close one of both

By Administrator on 2013-05-19T20:22:52 (imported from GitLab project)

By Administrator on 2013-05-19T20:22:52 (imported from GitLab)

Created by: bbodenmiller

Is this still an issue in 5.2 or latest master? Thanks for the issue report. Please reformat your issue to conform to the issue tracker guidelines found in our contributing guidelines.

By Administrator on 2013-06-14T01:07:57 (imported from GitLab project)

By Administrator on 2013-06-14T01:07:57 (imported from GitLab)

Created by: nielsbasjes

I still have it on version 5.2

By Administrator on 2013-06-15T08:13:51 (imported from GitLab project)

By Administrator on 2013-06-15T08:13:51 (imported from GitLab)

Created by: christianpauli

Same here. Is it possible to force the editor to utf8?

By Administrator on 2013-07-03T08:09:28 (imported from GitLab project)

By Administrator on 2013-07-03T08:09:28 (imported from GitLab)

Created by: docwhat

This is actually breaking things for us. We keep our puppet stuff in gitlab, but it breaks things if we ever edit stuff via gitlab.

By Administrator on 2013-07-12T17:15:32 (imported from GitLab project)

By Administrator on 2013-07-12T17:15:32 (imported from GitLab)

Created by: docwhat

I think the problem is with ACE. I tried updating to the latest ACE but it still doesn't work. I also tried monkeying with ACE directly and it isn't working either. sigh

By Administrator on 2013-07-12T19:46:53 (imported from GitLab project)

By Administrator on 2013-07-12T19:46:53 (imported from GitLab)

Created by: docwhat

@christianpauli FYI: It's not a UTF8 vs. some other encoding. Using \r\n is a valid way to EOL regardless of the encoding. But it's not valid for some text files.

In this case, it already is UTF-8.

By Administrator on 2013-07-12T22:56:37 (imported from GitLab project)

By Administrator on 2013-07-12T22:56:37 (imported from GitLab)

Created by: nielsbasjes

@docwhat More importantly; this causes the changes to show much more differences than intended.

By Administrator on 2013-07-13T10:53:37 (imported from GitLab project)

By Administrator on 2013-07-13T10:53:37 (imported from GitLab)

Created by: docwhat

You can always clone it and use -w with git diff to see the real changes Breaking stuff by adding \r is a much more egregious problem.

By Administrator on 2013-07-14T01:16:23 (imported from GitLab project)

By Administrator on 2013-07-14T01:16:23 (imported from GitLab)

Created by: docwhat

I have figured out what is going on:

When you POST a form, it uses the MIME type application/x-www-form-urlencoded. According to the W3 Form Content Types a browser should always encode newlines as \r\n.

So that's where the \r\n is coming in. I guess I never noticed before, even though I've been doing this for a while; probably because all the data was going into a DB, not files on disk.

So what we need is gitlab to detect the line-endings on the file and convert the line endings before writing it to the file.

So the controller code should look something like:

  edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, @project, @ref, @path)
  params[:commit_message].gsub!(/\r\n/, edit_file_action.detected_eol_string)
  updated_successfully = edit_file_action.commit!(
      params[:content],
      params[:commit_message],
      params[:last_commit]
    )

detected_eol_string would have something like this in it:

if contents =~ /\r\n/
  return "\r\n"
elif contents =~ /\r[^\n]/
  return "\r"
else
  return "\n"
end

I'm not sure EditFileAction is the right place to hang detected_eol_string, to be honest.

Ciao!

By Administrator on 2013-07-15T14:17:23 (imported from GitLab project)

By Administrator on 2013-07-15T14:17:23 (imported from GitLab)

Created by: docwhat

@randx I'm happy to submit a pull request for this.

Where should I hang the detected_eol_string method and which branch should I submit it to?

By Administrator on 2013-07-15T14:19:12 (imported from GitLab project)

By Administrator on 2013-07-15T14:19:12 (imported from GitLab)

Created by: nielsbasjes

For this I would still prefer the commiter to remain in control. So I would prefer a solution that 1: adds a "Line endings" option (next to the save button in the webeditor) and 2: the preselected "Line endings" option for that file is chosen with a set of conditionals like you propose.

By Administrator on 2013-07-15T14:44:55 (imported from GitLab project)

By Administrator on 2013-07-15T14:44:55 (imported from GitLab)

Created by: docwhat

Sure! I can see that

Line Endings:

• Windows -- :title => "Convert doc to use \r\n"
• Unix (current) -- :title => "Use default \n"
• Mac OS <= 9 -- :title => "Convert doc to use \r"

Ciao!

By Administrator on 2013-07-15T17:33:59 (imported from GitLab project)

By Administrator on 2013-07-15T17:33:59 (imported from GitLab)

Created by: dzaporozhets

@docwhat PR is welcome! detected_eol_string can be moved to separate class in lib/

By Administrator on 2013-08-02T12:27:08 (imported from GitLab project)

By Administrator on 2013-08-02T12:27:08 (imported from GitLab)

Created by: dzaporozhets

@docwhat I really want this fixed so ping me if you cant submit PR. then I code it by myself

By Administrator on 2013-08-02T12:28:10 (imported from GitLab project)

By Administrator on 2013-08-02T12:28:10 (imported from GitLab)

Created by: docwhat

@randx;

I spent a bit of time looking into this and the more I did, the less I think GitLab should be mucking around with this.

I spent a little time looking at how github might be doing it and I think what they are doing is setting git config --global core.autocrlf input for their account(s) on their servers.

So I propose instead that we either add a check in rake gitlab:check or in gitlab-shell's bin/check for this setting.

This will let git do the right thing instead of us spending lots of effort on it.

For finer tuning create a .gitattributes file in your repository to force line endings for the file types you need to have \n line endings.

For example for my puppet repository, it looks like this:

* text=auto

*.pp text eol=lf
*.md text eol=lf
*.rb text eol=lf
*.sh text eol=lf

*.bz2 binary

Here's some good help:

• https://help.github.com/articles/dealing-with-line-endings

TL;DR Workaround

Log in as your git user and run git config --global core.autocrlf input

By Administrator on 2013-08-02T20:37:12 (imported from GitLab project)

By Administrator on 2013-08-02T20:37:12 (imported from GitLab)

Created by: docwhat

@randx I'll leave it to you to add it to one of the check functions.

By Administrator on 2013-08-02T20:38:09 (imported from GitLab project)

By Administrator on 2013-08-02T20:38:09 (imported from GitLab)

Created by: dzaporozhets

@docwhat thank you. Fixed by 2d681c0d

By Administrator on 2013-08-03T08:35:48 (imported from GitLab project)

By Administrator on 2013-08-03T08:35:48 (imported from GitLab)

Created by: eddiemonge

These seem to have come back.

By Administrator on 2015-01-13T19:38:32 (imported from GitLab project)

By Administrator on 2015-01-13T19:38:32 (imported from GitLab)

Created by: paulproteus

One option here is that, if the HTTP encoding is ruining the JSON object's contents, the JSON object could encode \r and \n not as themselves, but instead as their Unicode values via the \u escaping.

This interaction between myself and nodejs probably illustrates what I'm suggesting more clearly:

> var newline = '\n';
undefined
> var othernewline = '\u000A';
undefined
> othernewline == newline
true
> othernewline === newline
true
> 

My claim is that the string '\u000A' won't get corrupted by HTTP, so the Javascript tool should turn newlines into that before submitting.

By Administrator on 2015-01-11T09:08:44 (imported from GitLab project)

By Administrator on 2015-01-11T09:08:44 (imported from GitLab)