Community
Participate
Working Groups
3.7. Apply Patch uses wrong line delimiter for new files when pasting via clipboard. 1. Start new workspace 2. Set default line delimiter for new text files to 'Unix' 3. Create a project (need to be able to apply a patch) 4. Copy the following patch to the clipboard: https://bugs.eclipse.org/bugs/attachment.cgi?id=205785 5. Start to apply the patch 6. In the wizard, see that the new file IOverviewRulerExtension.java has Windows line delimiters even though the workspace is configured to use 'Unix' line delimiters. Note that the attached patch itself uses 'Unix' line delimiters. When applying the patch via file or URL it uses 'Unix' line delimiters as expected.
Created attachment 205824 [details] Fix Tomek, let me know whether you like this. If so, I'll push to the repo.
Created attachment 205828 [details] Fix (previous patch was too big)
With your patch the file is saved with Unix line endings indeed, but the Review Patch page still shows CRLF when applying the patch from clipboard and LF when applying the same patch via URL. Wouldn't it be better to fix the preview and store the patch as the user sees it? PS. I would have started my comment with "I like the patch" but I have to be careful now (bug 315747, comment 17) ;) PS2. I bumped into this when following the steps - bug 361814.
(In reply to comment #3) > With your patch the file is saved with Unix line endings indeed, but the Review > Patch page still shows CRLF when applying the patch from clipboard and LF when > applying the same patch via URL. Wouldn't it be better to fix the preview and > store the patch as the user sees it? I was thinking of that too. The reason why I chose it as is: - depending on the segment count we'd need to recompute the preview - one can see the original LD / patch in the preview - at the current code change location I have the IFile at hand I can change the patch if you prefer to see it in the preview. Do you know by chance where we'd have to do the conversion if we also want it in the preview?
Note that the most natural place to add the code would be Hunk.getLineDelimiter(List), but compare.core does not depend on core.resources.
Created attachment 205881 [details] Previous Fix without possible NPE
Created attachment 205906 [details] Test I wanted to back up your patch with a test, but to my surprise it passed even without the patch applied. As it turned out, the culript here is the way Windows sets the clipboard content, ie it replaces LF with CRLF on the fly. In the test, the content is set directly, no conversion to CRLF, so the file content has expected LDs ie LF. I guess there is nothing we can do about, so I would go with your latest patch.
Created attachment 205907 [details] mylyn/context/zip
> I wanted to back up your patch with a test, but to my surprise it passed even > without the patch applied. As it turned out, the culript here is the way > Windows sets the clipboard content, ie it replaces LF with CRLF on the fly. In > the test, the content is set directly, no conversion to CRLF, so the file > content has expected LDs ie LF. I guess in that case the test could simply 1. set the workspace preference to e.g. 'Windows' LD 2. verify that the created file has Windows LDs
Created attachment 205909 [details] Test v02 (In reply to comment #9) > 1. set the workspace preference to e.g. 'Windows' LD > 2. verify that the created file has Windows LDs Good point, silly me. Now the test works as expected: fails without the fix, and passes when patched.
I'll push the changes (incl. test) once M3 has left the station.
Fixed in master: a350892ff42a4f70722f1a5f36ab98ec161f134c Committed Tomek's test in master: 9e817837eb2a032139e9d2ed3b4169b5728c630f
