Skip to content

Clean up host related code #1354

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Clean up host related code #1354

wants to merge 6 commits into from

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Jul 3, 2025

This removes the need to have options as part of the CompilerHost entirely, and is a cleanup we were thinking about ages ago.

The only notable change is that we now just use \n for the output newline on Windows, which I think is just fine.

Also clean up some other unused code in the host.

@Copilot Copilot AI review requested due to automatic review settings July 3, 2025 04:31
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR cleans up newline handling by removing the NewLine method and related options from host interfaces, standardizing on "\n" for all outputs, and simplifying compiler host creation signatures.

  • Removed all NewLine() methods and fields from host interfaces and implementations.
  • Updated calls to compiler.NewCompilerHost (and related helpers) to drop the compiler options argument.
  • Switched from sys.NewLine() and fmt.Fprint(…, …, sys.NewLine()) to literal "\n" and fmt.Fprintln where appropriate.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/project/host.go Removed NewLine() from ServiceHost interface
internal/project/project.go Dropped NewLine() method and interface reference
internal/execute/system.go Removed NewLine() from System interface
internal/execute/watcher.go Updated NewCompilerHost calls and replaced newline printing
internal/execute/watch.go Replaced fmt.Fprint + sys.NewLine() with fmt.Fprintln
internal/api/server.go Removed NewLine field and NewLine() implementation
Comments suppressed due to low confidence (1)

internal/api/server.go:99

  • [nitpick] The newLine field in the Server struct is no longer assigned or used after removing the NewLine method. Consider deleting this field to avoid dead code.
		cwd:                options.Cwd,

@jakebailey jakebailey marked this pull request as draft July 3, 2025 04:49
@jakebailey jakebailey marked this pull request as ready for review July 3, 2025 06:33
@jakebailey jakebailey marked this pull request as draft July 3, 2025 06:39
@jakebailey jakebailey changed the title Eliminate NewLine in host Clean up host related code Jul 3, 2025
@jakebailey jakebailey marked this pull request as ready for review July 3, 2025 06:54
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.

1 participant