"Single Line Uncomment" uncomments an extra line

[Yaron10]

STR:
Paste the following code into a new document and set the language to CSS.

:root
{
	/* --toolbarbutton-inner-padding: 6px !important; */
	/* --arrowpanel-border-radius: 0px !important; */
	/* --panel-border-radius: 0px !important; */
}

.tab-context-line { height: 30px !important;  flex: 1 !important; }

Select lines 3 and 4 (even without 4's EOL).
"Single Line Uncomment".

Result:
Line 5 is wrongly uncommented.
The selection is also wrong.

---

Notepad++ v8.4.8 (64-bit)
Build time : Jan 20 2023 - 14:41:09
Path : C:---notepad++.exe
Command Line :
Admin mode : ON
Local Conf mode : ON
Cloud Config : OFF
OS Name : Windows 10 Pro (64-bit)
OS Version : 20H2
OS Build : 19042.985
Current ANSI codepage : 1255
Plugins :
ComparePlus (1.1)
HTMLTag (1.3.6)
MenuIcons (2.0.2)
PythonScript (2)
_CustomizeToolbar (4.2)

[tcleaveland]

This also happens in 8.5.3. "Toggle Comment" behaves correctly.

[Yaron10]

@donho,

This issue is a major problem in real-life.
May I ask you to add it to your To-Do list? - Whenever you have the time and mood.

Thank you!

[donho]

@Yaron10
Is it a regression?
If yes, could you tell me the last version on which this feature works?

[Yaron10]

@donho,

I should have tested this earlier.
The issue was introduced in v8.3.1. v8.2.1 seems to work as expected.

Thank you for looking into it. I appreciate it.

[molsonkiko]

So I think this is just a problem with multi-line comments in general.

On a hunch, I have spent a few minutes testing various combinations of single-line and multi-line comments (in languages that support both like JavaScript) and came to the following unscientific conclusions:

1. It seems impossible to uncomment a mixture of single-line comments and multiline comments with a single operation.
2. When uncommenting multiline comments, the beginning and end of the selected region can change somewhat unpredictably. Probably the current algorithm is something like this:

while there are multi-line comments in the selected region:
    uncomment one multi-line comment in the selected region
    don't resize the selected region, or resize it incorrectly, because of some mistake in the implementation

Again, this might be wrong because I haven't had time to dig into the code, but I suspect that better accounting of how the selected region might resize based on the loss of comment start and end characters would probably fix the issues here.

An XML test file I came up with:

<foo>
<!-- before bar --> <bar></bar>  <!-- after bar -->
<!-- line after bar --> 
</foo>

A JavaScript test file I came up with:

//single line first
// single line second line
/* multiline */ /*next multiline */ // single line same line as multilines
/* multiline next line */
/* fourth multiline */
// single line after multilines

[Yaron10]

@molsonkiko,

Thank you for the input.

Dealing with various more complex cases would be appreciated.
The example in this issue is basic & common. :)

[Yaron10]

@donho,

castings fix the regression.
Could you please test it?

Thank you.

bool Notepad_plus::undoStreamComment(bool tryBlockComment)
{
	const TCHAR *commentStart;
	const TCHAR *commentEnd;
	const TCHAR *commentLineSymbol;

	generic_string symbolStart;
	generic_string symbolEnd;
	generic_string symbol;

	const int charbufLen = 10;
	TCHAR charbuf[charbufLen]{};

	bool retVal = false;

	Buffer * buf = _pEditView->getCurrentBuffer();
	//-- Avoid side-effects (e.g. cursor moves number of comment-characters) when file is read-only.
	if (buf->isReadOnly())
		return false;
	if (buf->getLangType() == L_USER)
	{
		UserLangContainer * userLangContainer = NppParameters::getInstance().getULCFromName(buf->getUserDefineLangName());
		if (!userLangContainer)
			return false;

		symbol = extractSymbol('0', '0', userLangContainer->_keywordLists[SCE_USER_KWLIST_COMMENTS]);
		commentLineSymbol = symbol.c_str();
		symbolStart = extractSymbol('0', '3', userLangContainer->_keywordLists[SCE_USER_KWLIST_COMMENTS]);
		commentStart = symbolStart.c_str();
		symbolEnd = extractSymbol('0', '4', userLangContainer->_keywordLists[SCE_USER_KWLIST_COMMENTS]);
		commentEnd = symbolEnd.c_str();
	}
	else
	{
		commentLineSymbol = buf->getCommentLineSymbol();
		commentStart = buf->getCommentStart();
		commentEnd = buf->getCommentEnd();
	}


	// BlockToStreamComment: If there is no stream-comment symbol and we came not from doBlockComment, try the block comment:
	if ((!commentStart) || (!commentStart[0]) || (commentStart == NULL) || (!commentEnd) || (!commentEnd[0]) || (commentEnd == NULL))
	{
		if (!(!commentLineSymbol || !commentLineSymbol[0] || commentLineSymbol == NULL) && tryBlockComment)
			return doBlockComment(cm_uncomment);
		else
			return false;
	}

	generic_string start_comment(commentStart);
	generic_string end_comment(commentEnd);
	generic_string white_space(TEXT(" "));
	size_t start_comment_length = start_comment.length();
	size_t end_comment_length = end_comment.length();

	_pEditView->execute(SCI_BEGINUNDOACTION);

	// do as long as stream-comments are within selection
	do
	{
		auto selectionStart = _pEditView->execute(SCI_GETSELECTIONSTART);
		auto selectionEnd = _pEditView->execute(SCI_GETSELECTIONEND);
		auto caretPosition = _pEditView->execute(SCI_GETCURRENTPOS);
		auto docLength = _pEditView->execute(SCI_GETLENGTH);

		// checking if caret is located in _beginning_ of selected block
		bool move_caret = caretPosition < selectionEnd;

		//-- Note: The caretPosition is either at selectionEnd or at selectionStart!! selectionStart is always before (smaller) than selectionEnd!!

		//-- First, search all start_comment and end_comment before and after the selectionStart and selectionEnd position.
		const int iSelStart=0, iSelEnd=1;
		const size_t N_CMNT = 2;
		intptr_t posStartCommentBefore[N_CMNT], posEndCommentBefore[N_CMNT], posStartCommentAfter[N_CMNT], posEndCommentAfter[N_CMNT];
		bool blnStartCommentBefore[N_CMNT], blnEndCommentBefore[N_CMNT], blnStartCommentAfter[N_CMNT], blnEndCommentAfter[N_CMNT];
		intptr_t posStartComment, posEndComment;
		intptr_t selectionStartMove, selectionEndMove;
		int flags;

		//-- Directly use Scintilla-Functions
		//   rather than _findReplaceDlg.processFindNext()which does not return the find-position and is not quiet!
		flags = SCFIND_WORDSTART;
		_pEditView->execute(SCI_SETSEARCHFLAGS, flags);
		//-- Find all start- and end-comments before and after the selectionStart position.
		//-- When searching upwards the start-position for searching must be moved one after the current position
		//   to find a search-string just starting before the current position!
		//-- Direction DIR_UP ---
		posStartCommentBefore[iSelStart] = _pEditView->searchInTarget(start_comment.c_str(), start_comment_length, selectionStart, 0);
		(posStartCommentBefore[iSelStart] == -1 ? blnStartCommentBefore[iSelStart] = false : blnStartCommentBefore[iSelStart] = true);
		posEndCommentBefore[iSelStart] = _pEditView->searchInTarget(end_comment.c_str(), end_comment_length, selectionStart, 0);
		(posEndCommentBefore[iSelStart] == -1 ? blnEndCommentBefore[iSelStart] = false : blnEndCommentBefore[iSelStart] = true);
		//-- Direction DIR_DOWN ---
		posStartCommentAfter[iSelStart] = _pEditView->searchInTarget(start_comment.c_str(), start_comment_length, selectionStart, docLength);
		(posStartCommentAfter[iSelStart] == -1 ? blnStartCommentAfter[iSelStart] = false : blnStartCommentAfter[iSelStart] = true);
		posEndCommentAfter[iSelStart] = _pEditView->searchInTarget(end_comment.c_str(), end_comment_length, selectionStart, docLength);
		(posEndCommentAfter[iSelStart] == -1 ? blnEndCommentAfter[iSelStart] = false : blnEndCommentAfter[iSelStart] = true);

		//-- Check, if selectionStart or selectionEnd is within a stream comment -----
		//   or if the selection includes a complete stream-comment!! ----------------

		//-- First, check if there is a stream-comment around the selectionStart position:
		if ((blnStartCommentBefore[iSelStart] && blnEndCommentAfter[iSelStart])
			&& (!blnEndCommentBefore[iSelStart] || (posStartCommentBefore[iSelStart] >= posEndCommentBefore[iSelStart]))
			&& (!blnStartCommentAfter[iSelStart] || (posEndCommentAfter[iSelStart] <= posStartCommentAfter[iSelStart])))
		{
				posStartComment = posStartCommentBefore[iSelStart];
				posEndComment   = posEndCommentAfter[iSelStart];
		}
		else //-- Second, check if there is a stream-comment around the selectionEnd position:
		{
			//-- Find all start- and end-comments before and after the selectionEnd position.
			//-- Direction DIR_UP ---
			posStartCommentBefore[iSelEnd] = _pEditView->searchInTarget(start_comment.c_str(), start_comment_length, selectionEnd, 0);
			(posStartCommentBefore[iSelEnd] == -1 ? blnStartCommentBefore[iSelEnd] = false : blnStartCommentBefore[iSelEnd] = true);
			posEndCommentBefore[iSelEnd] = _pEditView->searchInTarget(end_comment.c_str(), end_comment_length, selectionEnd, 0);
			(posEndCommentBefore[iSelEnd] == -1 ? blnEndCommentBefore[iSelEnd] = false : blnEndCommentBefore[iSelEnd] = true);
			//-- Direction DIR_DOWN ---
			posStartCommentAfter[iSelEnd] = _pEditView->searchInTarget(start_comment.c_str(), start_comment_length, selectionEnd, docLength);
			(posStartCommentAfter[iSelEnd] == -1 ? blnStartCommentAfter[iSelEnd] = false : blnStartCommentAfter[iSelEnd] = true);
			posEndCommentAfter[iSelEnd] = _pEditView->searchInTarget(end_comment.c_str(), end_comment_length, selectionEnd, docLength);
			(posEndCommentAfter[iSelEnd] == -1 ? blnEndCommentAfter[iSelEnd] = false : blnEndCommentAfter[iSelEnd] = true);

			if ((blnStartCommentBefore[iSelEnd] && blnEndCommentAfter[iSelEnd])
				&& (!blnEndCommentBefore[iSelEnd] || (posStartCommentBefore[iSelEnd] >= posEndCommentBefore[iSelEnd]))
				&& (!blnStartCommentAfter[iSelEnd] || (posEndCommentAfter[iSelEnd] <= posStartCommentAfter[iSelEnd])))
			{
					posStartComment = posStartCommentBefore[iSelEnd];
					posEndComment   = posEndCommentAfter[iSelEnd];
			}
			//-- Third, check if there is a stream-comment within the selected area:
			else if ( (blnStartCommentAfter[iSelStart] && (posStartCommentAfter[iSelStart] < selectionEnd))
				&& (blnEndCommentBefore[iSelEnd] && (posEndCommentBefore[iSelEnd] >  selectionStart)))
			{
					//-- If there are more than one stream-comment within the selection, take the first one after selectionStart!!
					posStartComment = posStartCommentAfter[iSelStart];
					posEndComment   = posEndCommentAfter[iSelStart];
			}
			//-- Finally, if there is no stream-comment, return
			else
			{
				_pEditView->execute(SCI_ENDUNDOACTION);
				return retVal;
			}
		}

		//-- Ok, there are valid start-comment and valid end-comment around the caret-position.
		//   Now, un-comment stream-comment:
		retVal = true;
		intptr_t startCommentLength = static_cast<intptr_t>(start_comment_length);
		intptr_t endCommentLength = static_cast<intptr_t>(end_comment_length);

		//-- First delete end-comment, so that posStartCommentBefore does not change!
		//-- Get character before end-comment to decide, if there is a white character before the end-comment, which will be removed too!
		_pEditView->getGenericText(charbuf, charbufLen, posEndComment-1, posEndComment);
		if (wcsnicmp(charbuf, white_space.c_str(), white_space.length()) == 0)
		{
			endCommentLength +=1;
			posEndComment-=1;
		}
		//-- Delete end stream-comment string ---------
		_pEditView->execute(SCI_BEGINUNDOACTION);
		_pEditView->execute(SCI_SETSEL, posEndComment, posEndComment + endCommentLength);
		_pEditView->execute(SCI_REPLACESEL, 0, reinterpret_cast<LPARAM>(""));

		//-- Get character after start-comment to decide, if there is a white character after the start-comment, which will be removed too!
		_pEditView->getGenericText(charbuf, charbufLen, posStartComment+startCommentLength, posStartComment+startCommentLength+1);
		if (wcsnicmp(charbuf, white_space.c_str(), white_space.length()) == 0)
			startCommentLength +=1;

		//-- Delete starting stream-comment string ---------
		_pEditView->execute(SCI_SETSEL, posStartComment, posStartComment + startCommentLength);
		_pEditView->execute(SCI_REPLACESEL, 0, reinterpret_cast<LPARAM>(""));
		_pEditView->execute(SCI_ENDUNDOACTION);

		//-- Reset selection before calling the routine
		//-- Determine selection movement
		//   selectionStart
		if (selectionStart > posStartComment)
		{
			if (selectionStart >= posStartComment+startCommentLength)
				selectionStartMove = -static_cast<intptr_t>(startCommentLength);
			else
				selectionStartMove = -static_cast<intptr_t>(selectionStart - posStartComment);
		}
		else
			selectionStartMove = 0;

		//   selectionEnd
		if (selectionEnd >= posEndComment+endCommentLength)
			selectionEndMove = -static_cast<intptr_t>(startCommentLength+endCommentLength);
		else if (selectionEnd <= posEndComment)
			selectionEndMove = -static_cast<intptr_t>(startCommentLength);
		else
			selectionEndMove = -static_cast<intptr_t>(startCommentLength + (selectionEnd - posEndComment));

		//-- Reset selection of text without deleted stream-comment-string
		if (move_caret)
		{
			// moving caret to the beginning of selected block
			_pEditView->execute(SCI_GOTOPOS, selectionEnd+selectionEndMove);
			_pEditView->execute(SCI_SETCURRENTPOS, selectionStart+selectionStartMove);
		}
		else
		{
			_pEditView->execute(SCI_SETSEL, selectionStart+selectionStartMove, selectionEnd+selectionEndMove);
		}
	}
	while (1); //do as long as stream-comments are within selection
}

[donho]

@Yaron10
Fabulous! Thank you for fixing it.
The fix has been committed into master: 7bbe4d1

[Yaron10]

@donho,

Great. Thank you for testing and committing. 👍