Skip to content

Conversation

@BDisp
Copy link
Collaborator

@BDisp BDisp commented Sep 16, 2022

Fixes #2022 - The contentBottomRightCorner was always drawing over the down arrow.

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@tig
Copy link
Collaborator

tig commented Sep 16, 2022

@BDisp I'm afraid my latest merges caused a conflict here. Can you resolve?

@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

@BDisp I'm afraid my latest merges caused a conflict here. Can you resolve?

Did you made this change? I'm getting an error:

imagem
imagem

@tig
Copy link
Collaborator

tig commented Sep 16, 2022

@BDisp I'm afraid my latest merges caused a conflict here. Can you resolve?

Did you made this change? I'm getting an error:

imagem imagem

I don't think I did. I am super confused myself!

@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

In my commit e8ef793 I only reformat that. But you did a revert and now the develop branch is different:


		public void ClearKeybinding (Command command)
		{
			foreach (var kvp in KeyBindings.Where (kvp => kvp.Value == command).ToArray ()) {
				KeyBindings.Remove (kvp.Key);
			}
		}

@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

Here it is a8b7f94

@tig
Copy link
Collaborator

tig commented Sep 16, 2022

How did I do that? Arrg.

So sorry.

Can you easily tell what the correct way to fix this is?

@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

How did I do that? Arrg.

So sorry.

Can you easily tell what the correct way to fix this is?

I think the better accurate way is doing a reset keeping the changes. Then you create a new PR with the title equal to the original. I think doing a revert on a reverted commit all the changes are lost. So do a reset keeping the changes.

@BDisp BDisp closed this Sep 16, 2022
@BDisp BDisp reopened this Sep 16, 2022
@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

If you already other commits after the reverted commit the reset isn't a good solution, because it will undo all the commits above. I'm googling about the better way :-)

@tig
Copy link
Collaborator

tig commented Sep 16, 2022

I have no idea where the vp.Value.SequenceEqual thing came from.

I've gone back and looked at the v1.7.x tags and that code was not there! What PR introduced this?

Is this the only line that is screwed up?

@BDisp BDisp force-pushed the scrollbarview-down-arrow-fix branch from e8ef793 to 583a8fb Compare September 16, 2022 17:35
@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

Is this the only line that is screwed up?

No, this PR was rebase from a wrong develop branch and that why this it having errors.

@tig
Copy link
Collaborator

tig commented Sep 16, 2022

@BDisp Would it be possible for you to recover the changes you meant for this PR from your local repo, create a new branch, and push a new PR?

@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

@BDisp Would it be possible for you to recover the changes you meant for this PR from your local repo, create a new branch, and push a new PR?

I'll try to found some original branch which I can recover with the latest changes.

@BDisp BDisp force-pushed the scrollbarview-down-arrow-fix branch from 3669b65 to dfcccff Compare September 16, 2022 18:38
@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

I already did with the latest branch which was already merged in PR #2020 and I still get this conflicts. I also doesn't seen in the develop branch the "ReflectionTools.cs" file which was merged with the PR #2000.
The changes in the View.ClearKeybinding was merged on the PR #1966.
I don't know what more I have to do.

@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

@BDisp Would it be possible for you to recover the changes you meant for this PR from your local repo, create a new branch, and push a new PR?

I already tried create a new branch to push a new PR, but result in one unchanged branch because indeed this changes were already merged no the PR #2020 to avoid unit test failure with the ScrollBarView on the ViewTests. The main problem now is the branch develop is screwed up. So I will close this PR because it was already merged and will be updated after the develop branch is fixed.

@BDisp BDisp closed this Sep 16, 2022
@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

Here is a print screen of all files that were changed with news PR's after the PR #2020 was merged:

imagem
imagem
imagem

@BDisp
Copy link
Collaborator Author

BDisp commented Sep 16, 2022

I think I discovery the culprit of the develop branch mess up. Is in the PR #2027 on the commit a45314e.
Probably you merged from main into develop branch with old content in the commit 959f8b3.

I think the better option to solve this is doing a git reset with keep changes option and stage all that is right and submit a new PR again. When all is well verified you can discard the wrongs changes.

@tig
Copy link
Collaborator

tig commented Sep 16, 2022

Bummer.

I won't be able to work on this until Sunday at the earliest!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScrollBarView doesn't show the down arrow on vertical if there isn't horizontal.

2 participants