--
Ticket URL: <www.midnight-commander.org/ticket/1652>
Midnight Commander <www.midnight-commander.org>
Midnight Development Center
--
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:1>
Comment(by birdie):
Or even better:
suggest or "guess" line endings upon saving a file (WIN/LIN/MACOS).
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:2>
--
Comment(by angel_il):
Replying to [comment:2 birdie]:
> Or even better:
>
> suggest or "guess" line endings upon saving a file (WIN/LIN/MACOS).
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:3>
--
Comment(by angel_il):
ups...
You mean #1571?
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:4>
Comment(by SzG):
If you implement an option, \r SHOULD BE SHOWN by default.
Mcedit is used as part of Cygwin by poor people who are forced to use
Windows in the office (like myself). On Windows not all editors are
preserving \n linefeeds like Far-manager, other editors may insert \r. The
best way to check it is mcedit.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:5>
* owner: => angel_il
* status: new => accepted
* version: 4.7.0-pre3 => master
* milestone: 4.7 => 4.7.1
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:6>
* milestone: 4.7.1 => Future Releases
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:7>
Comment(by sorin):
Currently this is one of the biggest issues with mc. The inability to work
with different line ending give problems in lots of cases. It is something
really common for see CR/LF even on linux from files coming from windows
share, ftp, source control.
Like other editor mc should be able to detect the line ending and keep it.
It could should the line ending in the status bar.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:8>
Comment(by zyv):
Oh really? How did you conclude that mc is *unable* to work with different
line endings?! It currently keeps the endings of edited files, however, \r
endings are not shown by default, whereas ^M are. So it's perfectly safe
to edit any kinds of files. Moreover, you can save files using any scheme
that you want.
All it takes is to add the ability to optionally display alien endings and
add options to select default endings scheme / and heuristics on keeping
auto-detected endings or auto-converting them to the default scheme.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:9>
* cc: zyv (added)
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:10>
* cc: go...@polanet.pl (added)
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:11>
* cc: pmi...@gmail.com (added)
Comment:
This is truly needed feature, and I lost my patience and went to implement
it myself. At first I thought that just making mcedit to not display
{{{^M}}} if followed by \n would be enough, but turned out that I'd need
to teach cursor to skip them too in various places, and that would be lot
of effort to learn mcedit's internals and do it right in all places.
So I indeed went for the following solution:
1. On opening file, detect line-endings used by sampling some initial
content.
1. If it happen to be CR or CRLF, skip fast load path, and in
edit_insert_file() convert such line endings to \n.
1. Save detected line ending type for editor.
That's basicly all - alien text files are converted to unix on load, then
converted vice-versa on save. Only extra thing needed on this stage was to
make Save As to treat "don't change" as indeed don't change (to not reset
auto-detected line ending type).
Patch is attached (note - debug output). I'd appreciate review for the
approach. Then, further work might be:
1. Add config setting for auto-detect behavior.
1. Add UI option for this setting.
1. Optionally, on loading file with autodected line breaks, verify that
all breaks indeed conform detected type, and otherwise warn user that one
may have non-reversible data change.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:12>
Comment(by pfalcon):
Replying to [comment:12 pfalcon]:
> 1. Add config setting for auto-detect behavior.
> 1. Add UI option for this setting.
> 1. Optionally, on loading file with autodected line breaks, verify that
all breaks indeed conform detected type, and otherwise warn user that one
may have non-reversible data change.
4. Display line endings used in status line.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:13>
Comment(by angel_il):
pfalcon:
ok, me like your patch.
1) can you create repo on github.com ?
2) i think no need add new param in edit_insert_file, just use edit->lb
for this
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:14>
Comment(by angel_il):
branch: [http://www.midnight-commander.org/changeset/1652_autodetect_lb
1652_autodetect_lb]
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:15>
* severity: no branch => on review
* milestone: Future Releases => 4.8.0-pre1
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:16>
Comment(by pfalcon):
Replying to [comment:14 angel_il]:
> pfalcon:
> ok, me like your patch.
> 1) can you create repo on github.com ?
Well, so far I do minimal hacking, so git diff/format-patch is less
effort. If I'll do more, I'll push to github (btw, is there some repo to
fork there? I cloned from midnight-commander.org)
> 2) i think no need add new param in edit_insert_file, just use edit->lb
for this
Well, that would mean loss of operation precision. What edit_insert_file()
does is inserts file ''filename'' with encoding ''lb'' into editor object
''edit'' with encoding ''edit->lb''. In general case, lb != edit->lb. Yes,
for what I implemented so far it doesn't matter, but "Insert file..." menu
can be made to take advantage of lb detection too. And if "Insert file..."
instead presume that inserted file's lb's are the same as currently being
edited, that sooner or later will lead to unexpected behavior for some
users, which then will complain that mc is buggy. So, worth doing it right
from the first time ;-).
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:17>
Comment(by pfalcon):
Updated patch to show current (non-LB_ASIS) encoding in status line.
Here's sample, still fits in 80 chars:
{{{
MainFrm.cpp ---- 0 87/87 1746/1746 <EOF> ASCII CRLF
100%
}}}
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:18>
Comment(by pfalcon):
Oops, I actually didn't check into 1652_autodetect_lb, where status line
feature was already added. Nevermind. I still think it's better to pass
LineBreaks to edit_insert_file explicitly.
And with changes to edit_get_save_file_as() in
3de1e2f26299017afa887f849199995c893de3e1, the whole option N_("&Do not
change"), should be removed, as user's choosing it will lead to file being
converted to LF, which is rather unexpected. But you cannot just remove
that choice from dialog, as it is tied to LB_ASIS being 0. So, either
LB_ASIS should be removed at all, or handled properly - ''not changing''
edit->lb, like my original patch did.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:19>
Comment(by pfalcon):
Also, 8d28ef0b4756b3af0177a59433bd1be347d3c274 shows LB only for
simple_statusbar case, probably worth adding for both types.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:20>
* severity: on review => on rework
Comment:
mceditor should be stay as binary-safe editor, therefore need to implement
configuration option for switching 'crlf autodetection' behaviour.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:21>
Comment (by angel_il):
please review
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:22>
* severity: on rework => on review
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:23>
Comment (by angel_il):
please review
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:24>
Comment (by andrew_b):
If '''autodect is off''', the editor has new behaviour that looks like a
bug.
1. Status line shows wrong line break.
How to reproduce:
1a. Open a text file with DOS line breaks.
Result: Status line shows LF.
2. Editor is not binary-safe now.
2a. Open a new text file pressing shift-f4.
2b. Create several lines
{{{
a
b
}}}
then save file with DOS line breaks (shift-f2 -> Windows/DOS format (CR
LF)) and close the editor.
2c. Open that file in editor. You can see !^M at line ends. That is ok.
2d. Move cursor to the first line, copy it pressing f3, down, f3.
2e. Save file pressing f2 (you still can see !^M at line ends) and close
editor.
2f. Open this file again.
Result:DOS line breaks are lost.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:25>
* severity: on review => on rework
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:26>
Comment (by andrew_b):
Replying to [comment:25 andrew_b]:
> 2. Editor is not binary-safe now.
Simplest testcase:
> 2a. Open a test file with DOS line breaks. You can see !^M at line ends.
> 2b. Save file pressing f2 (you still can see !^M at line ends) and close
editor.
> 2c. Open this file again.
> Result:DOS line breaks are lost.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:27>
Comment (by angel_il):
Replying to [comment:27 andrew_b]:
> Replying to [comment:25 andrew_b]:
> > 2. Editor is not binary-safe now.
>
> Simplest testcase:
>
> 2a. Open a test file with DOS line breaks. You can see !^M at line ends.
> 2b. Save file pressing f2 (you still can see !^M at line ends) and close
editor.
> 2c. Open this file again.
> Result:DOS line breaks are lost.
fix: 8c84722307e873a37aa88c1a6f5caaeae97421a9
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:28>
Comment (by slavazanko):
Have some new tests and have changed code in branch. Also, need to
describe new feature and editor optiond in man pages and WIKI.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:29>
* branch_state: => no branch
Comment:
Is this ticket stalled? I'm missing this feature. Can this be integrated
to main trunk?
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:30>
Comment (by pfalcon):
Well, I'm, as a contributor of the initial patch, found it perplexing that
my patch is rewritten for no apparent reason, and while that being done,
new bugs are added to it - exactly those which I could predict to happen
and deliberately avoided in my original patch. So, it's a strange
situation - I cannot continue with updated patch because I know it's
buggy, and my original patch was essentially thrown away. And nobody else
seem to continue with this either.
I guess, I should just swallow that and try again, because I like mc and
use it daily, and each time I see ^M there (and that happens often!) I
think to myself: "damn, I did patch to fix that, how on earth it happens I
still see that issue?"
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:31>
Comment (by slavazanko):
Thanks for notice.
Branch rebased to current master.
Changesets:
c98892acfb105ab64bdb95a4302c14539d4f2c2b Added unit tests for checking
"detect type of line breaks" functionality
a3cf368eab04029ffc1cbe036a5454500513ebd9 added configure option
editor_autodetect_linebreak
58aaddd431a3d2e9df4781913beffbe4bf76fd94 Ticket !#1652: autodetect line-
endings
Review, please.
pfalcon, as your original patch was changed, review and vote too, please.
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:32>
* milestone: 4.8.0-pre1 => Future Releases
--
Ticket URL: <www.midnight-commander.org/ticket/1652#comment:33>