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.

Sign up for GitHub

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

Jump to bottom

feat: Added Branches tree view #729

Merged
merged 19 commits into from
Nov 13, 2019
Merged

feat: Added Branches tree view #729

merged 19 commits into from
Nov 13, 2019

Conversation

FractalBoy
Copy link
Contributor

this adds a new tree view that gives you a nice listing of all the files that changed between the current branch and the branch it was copied from.

image

tooltip:

image

clicking something marked M will open up a diff:

image

@JohnstonCode JohnstonCode self-requested a review November 10, 2019 13:33
JohnstonCode
JohnstonCode requested changes Nov 10, 2019
View reviewed changes
@JohnstonCode
Copy link
Owner

Sorry to be a pain but can you get it to refresh when changes are detected like the repo log does. https://github.com/JohnstonCode/svn-scm/blob/master/src/historyView/repoLogProvider.ts#L122

@FractalBoy
Copy link
Contributor Author

the one feature i think maybe should be added before this is merged is a refresh button. do you agree? are there any other features that you think should be added?

@JohnstonCode
Copy link
Owner

Yes a refresh button would be useful. Apart from that i can think of anything else.

@FractalBoy
Copy link
Contributor Author

Sorry to be a pain but can you get it to refresh when changes are detected like the repo log does. https://github.com/JohnstonCode/svn-scm/blob/master/src/historyView/repoLogProvider.ts#L122

Not sure if I did this right...I don't know how to trigger the repo change event.

@JohnstonCode
Copy link
Owner

Yeah seems there is a loop. Let me have a look

@JohnstonCode
Copy link
Owner

the refresh in getChildren will just casue an infinate loop as its called by vscode

@FractalBoy
Copy link
Contributor Author

that's why i put in the "fire" boolean - before it was triggering refresh, then triggering getChildren, etc.

i don't think there's an infinite loop anymore - i just don't see the event being triggered at all when i set a breakpoint

@JohnstonCode
Copy link
Owner

Yeah sorry, the onDidChangeRepository fires once then it just keeps refreshing

@JohnstonCode
Copy link
Owner

Got it refresh -> repo.getChanges -> run -> updateModelState -> _onDidChangeStatus.fire -> this.model.onDidChangeRepository then it just repeats

@FractalBoy
Copy link
Contributor Author

hmmm...any suggestions?

@JohnstonCode
Copy link
Owner

Yeah give me a couple of minutes i will give you a patch

@JohnstonCode
Copy link
Owner

Is the https://github.com/JohnstonCode/svn-scm/pull/729/files#diff-d982bcb6ae46cbef9f548f44bd09324bR224 update needed?

@FractalBoy
Copy link
Contributor Author

Is the https://github.com/JohnstonCode/svn-scm/pull/729/files#diff-d982bcb6ae46cbef9f548f44bd09324bR224 update needed?

yeah, otherwise the repo may not be on the revision that was just committed (not sure why this is)

@JohnstonCode
Copy link
Owner

The information from svn info is taken directly from the .svn/entries file, and not from the server. This stems from Subversion's basic design of not requesting information from the repository server whenever possible.

@FractalBoy
Copy link
Contributor Author

so maybe i need to do svn info on the URL instead of on the WC?

@JohnstonCode
Copy link
Owner

That would work. Also you need to add Operation.Changes to isReadOnly as its only doing log under the hood

@JohnstonCode
Copy link
Owner

Actually it wont be because of the update...

@JohnstonCode
Copy link
Owner

The manual refresh button might do for the time being. This is a complicated problem

@JohnstonCode
Copy link
Owner

The revision on the branch if off by one it seems

Correct that it is just using the last revision on master and the new branch commit revision on the branch

@JohnstonCode
Copy link
Owner

it is working just not updating on future commits

@FractalBoy
Copy link
Contributor Author

yeah, i'm seeing that too.. not sure why that's happening since i'm getting the info from the URL

@JohnstonCode
Copy link
Owner

so to fix it i need to do svn up then restart code then it picks up the changes

@FractalBoy
Copy link
Contributor Author

i just need to stop relying on the cache

@FractalBoy
Copy link
Contributor Author

also passing the url wasn't working

@FractalBoy
Copy link
Contributor Author

should work now

@JohnstonCode
Copy link
Owner

Yes does appear to be working. Will have a proper look at it tomorrow. Rally good job though.

@FractalBoy
Copy link
Contributor Author

thanks!

@FractalBoy
Copy link
Contributor Author

oh, i think it's not working entirely for me since there's no .svn in the current folder. i need to modify the watcher to look at a parent path (if that's even possible)

@FractalBoy
Copy link
Contributor Author

Note that only files within the current workspace folders can be watched.

I guess i'll just have to deal with manually reloading since my directories are often laid out so the .svn directory is in a higher level directory

@FractalBoy
Copy link
Contributor Author

Note that only files within the current workspace folders can be watched.

I guess i'll just have to deal with manually reloading since my directories are often laid out so the .svn directory is in a higher level directory

I made some changes to use fs.watch if the WC root is outside the workspace since workspace.createFileSystemWatcher only monitors files in the workspace.

@FractalBoy
Remove unnecessary recursive: true
1116303 
It only works on Windows and Mac and is unneeded since wc.db is in the .svn directory
@FractalBoy
Fix tests, hopefully
aae3737
@JohnstonCode
Copy link
Owner

If i open a child directory it breaks.
image

when in reality only one file has changed
image

@FractalBoy
Copy link
Contributor Author

If i open a child directory it breaks.
image

when in reality only one file has changed
image

should be fixed

@JohnstonCode
Copy link
Owner

Yeah seems to be working.

@JohnstonCode JohnstonCode self-requested a review November 13, 2019 19:22
JohnstonCode
JohnstonCode approved these changes Nov 13, 2019
View reviewed changes
Copy link
Owner

@JohnstonCode JohnstonCode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JohnstonCode JohnstonCode changed the title add tree view for seeing all changes between a branch and its parent feat: Added Branches tree view Nov 13, 2019
@JohnstonCode JohnstonCode merged commit cecd185 into JohnstonCode:master Nov 13, 2019
JohnstonCode added a commit that referenced this pull request Nov 13, 2019
@JohnstonCode
chore(release): 2.1.0 [skip ci]
4c9b377 
# [2.1.0](v2.0.2...v2.1.0) (2019-11-13)

### Features

* Added Branches tree view ([#729](#729)) ([cecd185](cecd185))
@JohnstonCode
Copy link
Owner

🎉 This PR is included in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@FractalBoy
Copy link
Contributor Author

Woohoo! Thanks!

@FractalBoy FractalBoy deleted the branch-changes branch November 13, 2019 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnstonCode JohnstonCode JohnstonCode approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FractalBoy @JohnstonCode