Skip to content

Fix history entries navigation taking extra navigates in debug console#93125

Merged
isidorn merged 2 commits intomicrosoft:masterfrom
piraces:fix-history-extra-navigations
Mar 24, 2020
Merged

Fix history entries navigation taking extra navigates in debug console#93125
isidorn merged 2 commits intomicrosoft:masterfrom
piraces:fix-history-extra-navigations

Conversation

@piraces
Copy link
Contributor

@piraces piraces commented Mar 20, 2020

This PR fixes extra navigations done in History entries of debugger console, which cause problems navigating through the history, retrieving incorrect values.

This PR fixes #93102

@isidorn
Copy link
Collaborator

isidorn commented Mar 23, 2020

@piraces hi. Thanks a lot for providing this PR. I love that you added unit tests.
One issue I see with this approach is that the clients of the history have to change how they use it. And the debug console is not the only client (we also use it in most of the Input Boxes like Search and Extensions).
So idealy we would fix this without changing the API of the history. Can we just change the funtionality of the next and previous to handle this case and that we do not have to add hasPrevious and hasNext?

@piraces
Copy link
Contributor Author

piraces commented Mar 23, 2020

Hi @isidorn . For sure. I will make the changes only in the next and previous methods.
It is a good idea to mantain the hasPrevious and hasNext methods private and then use them in the public methods or I should do all the functionallity in the next and previous methods?

Thanks.

@isidorn
Copy link
Collaborator

isidorn commented Mar 23, 2020

@piraces I would try to do everything in the next and previous if possible. However I leave that up to you since it is personal preference.

@piraces piraces force-pushed the fix-history-extra-navigations branch from 21941fc to 07b8999 Compare March 23, 2020 18:47
@piraces
Copy link
Contributor Author

piraces commented Mar 23, 2020

@isidorn I have finally made the commented changes. If there is another problem, please let me know.
Thanks.

@isidorn
Copy link
Collaborator

isidorn commented Mar 24, 2020

Great PR, thanks for contributing. I have verified it nicely fixes the issue. Thus merging in.
🍻

@isidorn isidorn merged commit e97426b into microsoft:master Mar 24, 2020
@isidorn isidorn added this to the March 2020 milestone Mar 24, 2020
@piraces piraces deleted the fix-history-extra-navigations branch March 24, 2020 12:41
@github-actions github-actions bot locked and limited conversation to collaborators May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

History entries navigation takes one extra navigate on start and end of history

3 participants