Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Changed History: Go to Next/Prev block #12248

Closed
Yaron10 opened this issue Sep 26, 2022 · 33 comments
Closed

[Feature Request] Changed History: Go to Next/Prev block #12248

Yaron10 opened this issue Sep 26, 2022 · 33 comments
Assignees
Labels

Comments

@Yaron10
Copy link

Yaron10 commented Sep 26, 2022

"Changed History" margin was introduced in NPP 8.4.6.
Two menu commands "Go to Next/Prev block" would be a useful addition to that new feature.

Thank you.

@vinsworldcom
Copy link

A proof of concept in PythonScript:

def gotoLine(line):
    editor.setVisiblePolicy(CARETPOLICY.JUMPS | CARETPOLICY.EVEN, 0)
    editor.ensureVisibleEnforcePolicy(line)
    editor.gotoLine(line)
    editor.setVisiblePolicy(CARETPOLICY.EVEN, 0)
    editor.ensureVisibleEnforcePolicy(line)

def gotoNextChange(self):
    line = -1
    pos = editor.getCurrentPos()
    searchStart = editor.lineFromPosition(pos)

    ### `markerNext` doesn't work on ChangeHistory:
    #   https://sourceforge.net/p/scintilla/bugs/2353/
    lastLine = editor.lineFromPosition(editor.getTextLength())
    consec = searchStart
    for ln in range(searchStart+1, lastLine+1):
        if editor.markerGet(ln) & self._mask:
            if ln == consec + 1:
                consec += 1
            else:
                line = ln
                break

    if line == -1:
        for ln in range(0, searchStart):
            if editor.markerGet(ln) & self._mask:
                if ln == consec + 1:
                    consec += 1
                else:
                    line = ln
                    break

    if line != -1:
        gotoLine(line)

def gotoPrevChange(self):
    line = -1
    pos = editor.getCurrentPos()
    searchStart = editor.lineFromPosition(pos)

    while True:
        line = editor.markerPrevious(searchStart - 1, self._mask)

        if line == -1:
            break

        if line == (searchStart - 1):
            searchStart -= 1
        else:
            break

    if line == -1:
        end = editor.getLineCount()
        line = editor.markerPrevious(end, self._mask)

    if line != -1:
        gotoLine(line)

Cheers.

@Yaron10
Copy link
Author

Yaron10 commented Sep 26, 2022

@vinsworldcom,

Thank you for the nice script. 👍
Alas, it won't work for me. :)

@vinsworldcom
Copy link

vinsworldcom commented Sep 26, 2022

Alas, it won't work for me. :)

Sorry, it's a proof of concept - extracted from my larger working script. I can try to make is fully functional:

from Npp import editor, CARETPOLICY

class ChangeHistoryGoTo(object):
    def __init__(self):
        self._mask = (1 << 21) | (1 << 22) | (1 << 23) | (1 << 24)

    def _goto_line(self, line):
        editor.setVisiblePolicy(CARETPOLICY.JUMPS | CARETPOLICY.EVEN, 0)
        editor.ensureVisibleEnforcePolicy(line)
        editor.gotoLine(line)
        editor.setVisiblePolicy(CARETPOLICY.EVEN, 0)
        editor.ensureVisibleEnforcePolicy(line)

    def gotoNextChange(self):
        line = -1
        pos = editor.getCurrentPos()
        searchStart = editor.lineFromPosition(pos)

        ### `markerNext` doesn't work on ChangeHistory:
        #   https://sourceforge.net/p/scintilla/bugs/2353/
        lastLine = editor.lineFromPosition(editor.getTextLength())
        consec = searchStart
        for ln in range(searchStart+1, lastLine+1):
            if editor.markerGet(ln) & self._mask:
                if ln == consec + 1:
                    consec += 1
                else:
                    line = ln
                    break

        if line == -1:
            for ln in range(0, searchStart):
                if editor.markerGet(ln) & self._mask:
                    if ln == consec + 1:
                        consec += 1
                    else:
                        line = ln
                        break

        if line != -1:
            self._goto_line(line)

    def gotoPrevChange(self):
        line = -1
        pos = editor.getCurrentPos()
        searchStart = editor.lineFromPosition(pos)

        while True:
            line = editor.markerPrevious(searchStart - 1, self._mask)

            if line == -1:
                break

            if line == (searchStart - 1):
                searchStart -= 1
            else:
                break

        if line == -1:
            end = editor.getLineCount()
            line = editor.markerPrevious(end, self._mask)

        if line != -1:
            self._goto_line(line)

if __name__ == '__main__':
    chgt = ChangeHistoryGoTo()

Run from the PythonScript menu, then from the PythonScript console:

chgt.gotoNextChange()
chgt.gotoPrevChange()

Cheers.

@Yaron10
Copy link
Author

Yaron10 commented Sep 26, 2022

תמונה

I'm using PythonScript 2.

Thanks again. I appreciate that.
I wouldn't like to bother you with that.

@vinsworldcom
Copy link

I'm using PythonScript 2.

A-ha, I'm using 3.0.14

@Yaron10
Copy link
Author

Yaron10 commented Sep 26, 2022

Well, I'll wait for the integration of the feature in NPP.
And when I have time (and mood), I'll learn your script. :)

Thanks again.

@vinsworldcom
Copy link

And when I have time (and mood), I'll learn your script. :)

Try again, I removed the enum dependency.

@Yaron10
Copy link
Author

Yaron10 commented Sep 26, 2022

It works like a charm!
And you jump to the Next/Prev block. Very nice. 👍

Thank you.

@donho donho self-assigned this Sep 28, 2022
@vinsworldcom
Copy link

And you jump to the Next/Prev block. Very nice. 👍

Yes, that's key, otherwise it'll stop at every change line and if there are 20 new lines inserted in a row, you just want to stop at the top (next) or bottom (previous) - not each and every one.

Cheers.

@Yaron10
Copy link
Author

Yaron10 commented Sep 28, 2022

@vinsworldcom,

Thanks again for the beautiful script. 👍

Slightly adapted for my personal use. :)

# Based on https://github.com/notepad-plus-plus/notepad-plus-plus/issues/12248#issuecomment-1258561261.

class ChangeHistoryGoTo():
    def __init__(self):
        self._mask = (1 << 21) | (1 << 22) | (1 << 23) | (1 << 24)
        self._currentLine = editor.lineFromPosition(editor.getCurrentPos())

    def _goto_line(self, line):
        if line != -1 and line != self._currentLine:
            editor.ensureVisibleEnforcePolicy(line)
            editor.gotoLine(line)
        else:
            import winsound
            winsound.MessageBeep()

#----------------#

    def gotoNextOrFirstChange(self, gotoFirst):
        line = -1
        searchStartLine = self._currentLine
        lastLine = editor.getLineCount()

        if not gotoFirst:
            for searchLine in range(self._currentLine, lastLine):   # Start from currentLine (not currentLine + 1) in case currentLine is not-changed and the next line IS changed.
                if editor.markerGet(searchLine) & self._mask:
                    if searchLine != searchStartLine:      # changed-line found in a different block.
                        line = searchLine
                        break
                    else:
                        searchStartLine += 1

        if line == -1:
            for searchLine in range(0, lastLine if gotoFirst else self._currentLine):     # Wrap around.
                if editor.markerGet(searchLine) & self._mask:
                    line = searchLine
                    break

        self._goto_line(line)

#----------------#

    def gotoPrevChange(self):
        line = -1
        searchStartLine = self._currentLine

        while True:
            line = editor.markerPrevious(searchStartLine, self._mask)
            if line == -1 or line != searchStartLine:   # "line == -1": no changed-line found. "line != searchStartLine": changed-line found in a different block.
                break
            else:
                searchStartLine -= 1

        if line == -1:
            line = editor.markerPrevious(editor.getLineCount(), self._mask)     # Wrap around.

        self._goto_line(line)

#-------------------------------------------------------------------------------#

changeHistory = ChangeHistoryGoTo()
if clickButton == 0:
	changeHistory.gotoNextOrFirstChange(False)      # Next.
elif clickButton == 1:
	changeHistory.gotoNextOrFirstChange(True)       # First.
else:
	changeHistory.gotoPrevChange()

@donho
Copy link
Member

donho commented Jul 1, 2023

@Yaron10
Could you describe the behaviour of "Go to Next/Prev block"?
Is it based on cursor's positions?
Should it be in the same document?

@Yaron10
Copy link
Author

Yaron10 commented Jul 1, 2023

@donho,

Is it based on cursor's positions?

Yes, it is.

Should it be in the same document?

Yes, I think it should be per file.

And we should jump from block to block:
If lines 10-12 are changed, and the caret is at line 10 - we shouldn't go to line 11 but to the next block if exists.

I've implemented it in C++. Please ignore my ::MessageBeep() calls. I find them helpful. :)

// Based on https://github.com/notepad-plus-plus/notepad-plus-plus/issues/12248#issuecomment-1258561261.
void Notepad_plus::changedHistoryGoTo(int idGoTo)
{
	int mask = (1 << 21) | (1 << 22) | (1 << 23) | (1 << 24);
	intptr_t line = -1;
	intptr_t blockIndicator = _pEditView->getCurrentLineNumber();
	intptr_t lastLine = _pEditView->execute(SCI_GETLINECOUNT);

	if (idGoTo != IDM_SEARCH_CHANGED_PREV)		// Next or First.
	{
		intptr_t currentLine = blockIndicator;

		if (idGoTo == IDM_SEARCH_CHANGED_NEXT)
		{
			// Start from currentLine (not currentLine + 1) in case currentLine is not-changed and the next line IS changed. lastLine is at least *1*.
			for (intptr_t i = currentLine; i < lastLine; i++)
				if (_pEditView->execute(SCI_MARKERGET, i) & mask)
				{
					if (i != blockIndicator)		// Changed-line found in a different block.
					{
						line = i;
						break;
					}
					else
						blockIndicator++;
				}
		}

		if (line == -1)		// Wrap around.
		{
			intptr_t endRange = (idGoTo == IDM_SEARCH_CHANGED_NEXT) ? currentLine + 1 : lastLine;		//	"+ 1": currentLine might be *0*.
			for (intptr_t i = 0; i < endRange; i++)
				if (_pEditView->execute(SCI_MARKERGET, i) & mask)
				{
					line = i;
					if (idGoTo == IDM_SEARCH_CHANGED_NEXT)
						::MessageBeep(MB_OK);
					else if (i == currentLine)
						::MessageBeep(MB_ICONINFORMATION);

					break;
				}
		}
	}
	else	// Prev.
	{
		while (true)
		{
			line = _pEditView->execute(SCI_MARKERPREVIOUS, blockIndicator, mask);
			// "line == -1": no changed-line found. "line != blockIndicator": changed-line found in a different block.
			if (line == -1 || line != blockIndicator)
				break;
			else
				blockIndicator--;
		}

		if (line == -1)	// Wrap around.
		{
			line = _pEditView->execute(SCI_MARKERPREVIOUS, lastLine - 1, mask);
			if (line != -1)
				::MessageBeep(MB_OK);
		}
	}

	if (line != -1)
	{
		_pEditView->execute(SCI_ENSUREVISIBLEENFORCEPOLICY, line);
		_pEditView->execute(SCI_GOTOLINE, line);
	}
	else
		::MessageBeep(MB_ICONEXCLAMATION);
}

@donho donho added the accepted label Jul 2, 2023
@donho
Copy link
Member

donho commented Jul 2, 2023

@Yaron10

Two menu commands "Go to Next/Prev block" would be a useful addition to that new feature.

I would say "Go to Previous Change" & "Go to Next Change" - what do you think?

I will integrate your code and do a PR - if you still don't want to do a PR.

@Yaron10
Copy link
Author

Yaron10 commented Jul 2, 2023

@donho,

If you're planing to place the commands directly under Search, "Go to Next/Prev Change" would be better.
If the commands should be under a "Change History" popup (before or after "Bookmarks"?), "Go to Next/Prev Block" might be better.
As you understand.

It's @vinsworldcom's code. I'd be glad if you could do the PR.

Thank you!

@vinsworldcom
Copy link

vinsworldcom commented Jul 2, 2023 via email

@donho
Copy link
Member

donho commented Jul 2, 2023

Sorry, away this weekend no computer access. Sounds like the example would be from https://github.com/vinsworldcom/nppChangedLines

@vinsworldcom
Nice plugin! Is there any reason for Changed Lines plugin not being included in NppPluginList ?

@vinsworldcom
Copy link

vinsworldcom commented Jul 2, 2023 via email

@donho
Copy link
Member

donho commented Jul 3, 2023

@vinsworldcom

I think it's still worth to add it into plugin admin. Do you mind to add it?

@alankilborn
Copy link
Contributor

The Changed Lines plugin predates "Change History" in Scintilla/Notepad++. Now that change history is here, the plugin offers too much redundancy and should be retired. I believe @vinsworldcom agrees. Spreading the word about the plugin, i.e., offering via Plugins Admin is IMO a bad idea.

@donho
Copy link
Member

donho commented Jul 3, 2023

@vinsworldcom @alankilborn
OK, I see your point. Though I think it's still interesting for users to have a list of their modification content. Also users can install/uninstall if they like/dislike this feature - that's the whole point of plugin.
Anyway, NppPluginList is opened for all plugin authors, and it's the decision of plugin authors to make their plugins available or not in Plugin Admin.

@Yaron10
Copy link
Author

Yaron10 commented Jul 3, 2023

There's a nice "Clear Change History" command in @vinsworldcom's plugin.
I'm wondering if it can be easily implemented in NPP.

@alankilborn
Copy link
Contributor

interesting for users to have a list of their modification content

Then this would be a good idea for a feature within Notepad++ itself. :-)

@donho
Copy link
Member

donho commented Jul 3, 2023

@Yaron10

I'm wondering if it can be easily implemented in NPP.

If such feature can be in Change Lines plugin, it can be built-in feature in Notepad++.
The question is, where should we put it?

@Yaron10
Copy link
Author

Yaron10 commented Jul 3, 2023

@donho,

If such feature can be in Change Lines plugin, it can be built-in feature in Notepad++.

Great. 👍

The question is, where should we put it?

How about putting it in a "Change History" popup under Search?
"Go to Next Change".
"Go to Previous Change".
Separator.
"Clear History of the Current Document".

Thank you.

@Yaron10
Copy link
Author

Yaron10 commented Jul 3, 2023

Another question:
Should those commands be disabled if the History margin is not displayed?
Or just doing nothing (or beep :) ) would be enough?

@donho
Copy link
Member

donho commented Jul 3, 2023

Should those commands be disabled if the History margin is not displayed?
Or just doing nothing (or beep :) ) would be enough?

I'll see what I can do about it.

@alankilborn
Copy link
Contributor

How about putting it in a "Change History" popup under Search?

popup = submenu

Should those commands be disabled if the History margin is not displayed?

Disabled makes good sense. Beep is OK, but disabled is better IMO.

@Yaron10
Copy link
Author

Yaron10 commented Jul 3, 2023

@donho,

I'll see what I can do about it.

👍
Thank you.

I just want to point out that the Bookmarks commands are enabled even if the margin is not displayed.
So returning and doing nothing is also an option. - Not as good as disabled though.

@alankilborn,

Beep is OK, but disabled is better IMO.

I was joking. I knew you and Don wouldn't like the beep.

@alankilborn
Copy link
Contributor

Bookmarks commands are enabled even if the margin is not displayed

Are some new issues going in? :-)

I'd say that the proposed new features should be done correctly from the start, i.e., don't use existing bad behavior as something to align with.

@Yaron10
Copy link
Author

Yaron10 commented Jul 3, 2023

@alankilborn,

I'd say that the proposed new features should be done correctly from the start

👍

Actually my analogy was completely wrong: you can create and go-to bookmarks even if the margin is not displayed.


"Clear History of the Current Document" or "Clear Current Document History"?

@vinsworldcom
Copy link

Clear Change History

Just be aware that to do that, you lose your undo buffer.

@vinsworldcom
Copy link

The Changed Lines plugin predates "Change History" in Scintilla/Notepad++. Now that change history is here, the plugin offers too much redundancy and should be retired. I believe @vinsworldcom agrees.

The plugin does a little few things beyond the existing Notepad++ support for Scintilla Change History - that's why I kept it. I did strip out the entire change tracking that was implemented. The "extras" are:

  • go to last position, a bit hacky but records the position and lets you return to it (not necessarily "change history" related)
  • enable the indicators visuals for change history
  • change the colors for change history margin
  • clear change history on current doc
  • goto next / prev change (as discussed in this thread)
  • provide a right-click context menu in the change history margin for goto prev/next and clear
  • double click / shift d-click the change history margin to goto prev/next change
  • differentiate between saved vs. non-saved changes with goto prev/next

All of this is described in the README. Most could be done with PythonScripts. Not sure how much appetite there is for porting any of this "natively" to Notepad++. I like the extra features and use them so will continue to use the plugin as more of an add-on to existing capability (something I usually do with PythonScripts) vs. adding a brand new unique capability - as this did before Scintilla Change History.

As I said, the original code was "borrowed" from Location Navigate plugin, but since change tracking is out of the plugin now - most of the remaining code I wrote and you can do with it what you like. Happy to see some of all of this move to Notepad++.

Cheers.

@donho
Copy link
Member

donho commented Jul 3, 2023

Thanks to Change Lines plugin's feature Clear Change History, the bug regarding Reload from Disk (#12319, #12261 & #13735) is fixed now.
For people who are interested in it, check PR #13858

donho added a commit to donho/notepad-plus-plus that referenced this issue Jul 5, 2023
Add Go to next/prev change & clear change history commands.

Fix notepad-plus-plus#12248
donho added a commit to donho/notepad-plus-plus that referenced this issue Jul 5, 2023
Add Go to next/prev change & clear change history commands.

Fix notepad-plus-plus#12248
@donho donho closed this as completed in d9b9868 Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants