8000 [#314] fix broken API by ghentschke · Pull Request #316 · eclipse-cdt/cdt-lsp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[#314] fix broken API #316

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
Merged
merged 2 commits into from
May 13, 2024
Merged

[#314] fix broken API #316

merged 2 commits into from
May 13, 2024

Conversation

ghentschke
Copy link
Contributor

MyClangdOptionsDefaults has to implement ClangdOptionsDefaults instead of extending the now non-API class BuiltinClangdOptionsDefaults

fixes #314

@merks
Copy link
Contributor
merks commented May 13, 2024

@ghentschke

FYI, hopefully these changes fix the target platform problems:

#315

But someone needs to start the verification build....

Note that I put a comment here about the specific changes to the example:

ghentschke@5787464#commitcomment-141943807

@ruspl-afed
Copy link
Member

@ghentschke for some reason I do not see an option to rebase this PR, most probably it will be green after rebasing

@merks
Copy link
Contributor
merks commented May 13, 2024

@ghentschke @ruspl-afed

FYI, I expect if this is rebased on master that the build will start working again...

@ruspl-afed
Copy link
Member

FYI, I expect if this is rebased on master that the build will start working again...

So do I, but I do not see "Rebase" control in UI for this PR

@ghentschke
[eclipse-cdt#314] fix broken API
d985f97 
MyClangdOptionsDefaults has to implement ClangdOptionsDefaults instead
of extending the now non-API class BuiltinClangdOptionsDefaults

fixes eclipse-cdt#314
@ghentschke ghentschke force-pushed the fix-broken-example branch from 5787464 to d985f97 Compare May 13, 2024 09:59
ruspl-afed
ruspl-afed reviewed May 13, 2024
View reviewed changes
Copy link
Member
@ruspl-afed ruspl-afed left a comment

Choose a reason for hiding this comment

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

It will compile, but may be we can clean it up a bit:

  • provide reasonable default values
  • remove // TODO s

@merks
Copy link
Contributor
merks commented May 13, 2024

I think copying and pasting this would be better:

	@Override
	public String clangdPath() {
		return Optional.ofNullable(PathUtil.findProgramLocation("clangd", null)) //$NON-NLS-1$
				.map(IPath::toOSString)//
				.orElse("clangd"); //  //$NON-NLS-1$
	}

	@Override
	public boolean useTidy() {
		return true;
	}

	@Override
	public boolean useBackgroundIndex() {
		return true;
	}

	@Override
	public String completionStyle() {
		return "detailed"; //$NON-NLS-1$
	}

	@Override
	public boolean prettyPrint() {
		return true;
	}

	@Override
	public String queryDriver() {
		return Optional.ofNullable(PathUtil.findProgramLocation("gcc", null)) //$NON-NLS-1$
				.map(p -> p.removeLastSegments(1).append(IPath.SEPARATOR + "*"))// //$NON-NLS-1$
				.map(IPath::toString)//
				.orElse(""); //  //$NON-NLS-1$
	}

ruspl-afed
ruspl-afed approved these changes May 13, 2024
View reviewed changes
Copy link
Member
@ruspl-afed ruspl-afed left a comment

Choose a reason for hiding this comment

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

LGTM

@ruspl-afed
Copy link
Member

Thanks a lot @ghentschke ! Merging this one.

@ruspl-afed ruspl-afed merged commit 7366c10 into eclipse-cdt:master May 13, 2024
3 checks passed
@ghentschke ghentschke deleted the fix-broken-example branch May 13, 2024 12:42
Sign up for free to j 6629 oin this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruspl-afed ruspl-afed ruspl-afed approved these changes

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

Successfully merging this pull request may close these issues.

The class org.eclipse.cdt.lsp.examples.preferences.MyClangdOptionsDefaults doesn't compile
3 participants
@ghentschke @merks @ruspl-afed
0