Skip to content

Fix the sync issue with non-java files #131

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

Merged
merged 9 commits into from
Dec 21, 2018
Merged

Fix the sync issue with non-java files #131

merged 9 commits into from
Dec 21, 2018

Conversation

andxu
Copy link
Contributor

@andxu andxu commented Dec 20, 2018

No description provided.

@Flanker32 Flanker32 requested a review from yaohaizh December 21, 2018 01:14
}

public static IJarEntryResource createJarResource(URI uri) throws JavaModelException {
String handleId = uri.getQuery();
Copy link
Contributor

Choose a reason for hiding this comment

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

Null check or compatible check for URI getQuery here.

return uri != null && JDT_SCHEME.equals(uri.getScheme()) && CONTENTS_AUTHORITY.equals(uri.getAuthority());
}

public static IJarEntryResource createJarResource(URI uri) throws JavaModelException {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function name is misleading. You didn't create any resource here. The function just enumerate all resource files and return the list.

}
}
if (resource instanceof JarEntryDirectory) {
JarEntryDirectory directory = (JarEntryDirectory) resource;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the first one failed, it will not continue the search in other folders. Is this by design?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

public static IPath removeProjectSegment(String projectElementName, IPath path) {
if (projectElementName.equals(path.segment(0))) {
return path.removeFirstSegments(1).makeRelative();
}
return path;
}

public static PackageNode createNodeForProject(IJavaElement javaElement) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why all these functions are in the utility class? The file will be bloated with static methods. Please organize the logic clearly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return projectNode;
}

public static PackageNode createNodeForResource(IResource resource) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep the resource operation in well organized manner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

return result;
}


/**
* Create the node list from buttom to top until project
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: buttom ==> bottom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

* @return
* @throws JavaModelException
*/
private static List<PackageNode> getParentListToProject(IResource element) throws JavaModelException {
Copy link
Contributor

Choose a reason for hiding this comment

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

not parent list, but ancestor or path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

* @return
* @throws JavaModelException
*/
private static List<PackageNode> getParentAncestorNodes(IResource element) throws JavaModelException {
Copy link
Contributor

Choose a reason for hiding this comment

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

getAncestors is enough

@andxu
Copy link
Contributor Author

andxu commented Dec 21, 2018

#106

@andxu andxu merged commit aefa4e9 into master Dec 21, 2018
@andxu andxu deleted the andy_fix_sync branch December 21, 2018 06:23
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.

2 participants