Fri 08 Dec 2017 08:20:00 PM UTC, comment #11:
I meant "bug": bug #51438.
|
Sat 22 Jul 2017 08:00:28 PM UTC, comment #10:
The opposite bug has been filed as buf #51438.
|
Tue 11 Jul 2017 07:23:55 AM UTC, comment #9:
So, we've agreed that the bug is the opposite one: "without --tabstospaces, indenting via M-} adds a non-fixed amount of whitespace".
|
Fri 07 Jul 2017 07:37:33 PM UTC, comment #8:
Okay; I figured as much.
As for undoing a mixed unindent, the comment code saves the comment string in u->strdata in case the filename (and hence the associated comment) changes. I'm still trying to wrap my head around the undo code, but I suppose the indent code, when it's hooked up to undo/redo, could save the tab's worth of characters that gets added to or removed from the line in u->strdata, and the undo/redo code could use it to make sure that all indents are saved and restored properly. (This would also need to be done in case the TABS_TO_SPACES flag changes.)
But I should really take my comments to the other bug now :)
|
Fri 07 Jul 2017 06:05:01 PM UTC, comment #7:
For unindenting, the type of whitespace should not matter, as long as all selected lines can be unindented by a full tab's worth. (If not all lines can be unindented, M-{ should do nothing.)
Not caring about the type of whitespace makes it possible to quickly normalize the indentation of a piece of text to all spaces or all tabs (not considering partial indents) by selecting the text, doing M-{ a few times, and then doing M-} the required number of times. That is quicker, I think, than having to resort to search-and-replace.
(The other way around: caring about the type of whitespace would frustrate the user when he/she does not realize there is mixed whitespace in the file. Very annoying.)
Oof... but that will mean that undoing such a mixed unindent will not restore the situation exactly as it was before, because all the undo will do is add back a tab (or a tab's worth of spaces) to all the lines that were selected. Hmmm... Can't be helped. It's better than the current state: no undo available at all.
|
Fri 07 Jul 2017 04:53:31 PM UTC, comment #6:
After some thought, I'm in favor of the beginning for both indent and unindent. However, I thought I'd fix the existing behavior before changing it, which is why I made this patch first.
And I'm looking into cleaning up the function a bit before anything else, as I don't remember what I was thinking when I designed it to use the cols/-cols method to determine whether to indent or unindent.
The one real question I have relates to unindent. Currently, it doesn't care what whitespace it removes, as long as it takes up the same number of columns as an indent would (the --tabstospaces option makes the contents of a given indent vary, after all). And your comments in the other bug indicate that nano should ignore partial indents, which I'd be in favor of too.
So... what should your ideal unindent do if a block of text has full indents, but they're of different types? Say the block we've highlighted contains two lines: the first is indented with a tab, the second is indented with a tab's worth of spaces. Should unindent work on both lines, or should it refuse unless both lines have not just full indents, but the same type of full indents (both tabs, or both tabs' worth of spaces)?
And yes, this should definitely go into that other bug, and I'll put it in there after I get an answer to this. I just need to know how to proceed.
|
Fri 07 Jul 2017 04:28:02 PM UTC, comment #5:
The question is: should indenting add a tabsize worth of whitespace at the end of the line's leading whitespace, or at the beginning? The current method is weird, because it will add a tab after spaces (when tabstopaces is not set), which a) is bad form, and b) it doesn't preserve the relative indentation if some lines are indented by four spaces and others by a tab, for example.
If I would apply your patch, I would effectively strengthen the "wrong" method of indenting. I think that instead the opposite patch is needed, so that, also when tabstospaces is unset, an M-} will increase the visual identation of all selected lines by a full tab width.
(A secondary reason why I want it that way is that it is far easier to undo: just add or remove again one tab (or the corresponding number of spaces) to or from the head of each selected line (unless the line is empty), similar to what is done for commenting/uncommenting. When wanting to undo the current method of indenting, and the selected lines are each indented by a different number of spaces, how are we going to remember how many spaces were added to which line (when tabstospaces is set)? That requires a lot of bookkeeping.)
In short: I'm going to hold off on this patch.
|
Fri 07 Jul 2017 02:11:54 PM UTC, comment #4:
Updated version attached. And I'll definitely look at that other bug.
(file #41148)
|
Fri 07 Jul 2017 11:03:22 AM UTC, comment #3:
By the way, maybe have a look at bug #47684 too, and maybe comment there how /you/ think that indenting/unindenting should work when there are partial indents.
|
Fri 07 Jul 2017 10:26:28 AM UTC, comment #2:
Patch doesn't apply. You changed some braces in the meantime. Please provide an updated patch against master.
|
Fri 07 Jul 2017 06:07:42 AM UTC, comment #1:
Forget the last patch; the calculation was off since it relied on the current x position. The attached new patch properly calculates the number of spaces to indent per line when multiple lines are selected.
(file #41145)
|
Fri 07 Jul 2017 05:42:55 AM UTC, original submission:
Indenting via Meta-} is supposed to add a tab or the equivalent to the indentation already on a line. However, when the --tabstospaces option is used, Meta-} always adds tabsize's worth of spaces to that place, instead of adding the number of spaces that pressing Tab would add. This can be reproduced as follows:
1. Run nano with a new file:
./nano --ignore
2. On the first line, press Space twice, and then press Meta-}. The line will now contain two spaces and a tab, for a total of 8 columns, not counting the newline.
3. Press Enter to make a second line.
4. Press Meta-Q to turn on the --tabstospaces option.
5. On the second line, press Space twice, and then press Meta-}. The line will now contain two spaces and eight spaces, for a total of 10 columns, not counting the newline.
The Meta-} in step 5 should have added six spaces, not eight, in order to account for what a tab would take up relative to the current cursor position. The attached patch fixes this.
|