move ui and commands to helix-view #5581
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pascalkuthe
added
A-helix-term
pascalkuthe
force-pushed
the
refactor_commands
branch
2 times, most recently
from
7c80092 to
e8d440e
Compare
archseer
reviewed
Jan 19, 2023
pascalkuthe
force-pushed
the
refactor_commands
branch
from
e8d440e to
52570e7
Compare
|
@archseer I kept this PR minimal on purpose so right now it doesn't hide terminal specific code behind cfg attributes. I wasn't 100% sure what the best way to do that would be and wanted to leave that design space for when we actually implement guis. This PR already unblocks the non-gui related work. Are you okay with that or do you want me to add the cfg attributes in this PR? |
Closed
Closed
Merged
dead10ck
approved these changes
Jun 23, 2023
gabydd
reviewed
Aug 4, 2023
Comment on lines
-50
to
-52
| /// Render the component onto the provided surface. | ||
| fn render(&mut self, area: Rect, frame: &mut Surface, ctx: &mut Context); | ||
|
|
| @@ -264,14 +264,14 @@ impl Markdown { | |||
| CodeBlockKind::Fenced(language) => language, | |||
| CodeBlockKind::Indented => "", | |||
| }; | |||
| let tui_text = highlighted_code_block( | |||
| let helix_tui_text = highlighted_code_block( | |||
There was a problem hiding this comment.
this and the one below don't need to be changed
Suggested change
| let helix_tui_text = highlighted_code_block( | |
| let tui_text = highlighted_code_block( |
| text.to_string(), | ||
| language, | ||
| theme, | ||
| Arc::clone(&self.config_loader), | ||
| None, | ||
| ); | ||
| lines.extend(tui_text.lines.into_iter()); | ||
| lines.extend(helix_tui_text.lines.into_iter()); |
There was a problem hiding this comment.
Suggested change
| lines.extend(helix_tui_text.lines.into_iter()); | |
| lines.extend(tui_text.lines.into_iter()); |
|
|
||
| # TODO: graphics.rs tests rely on this, but we should remove that | ||
| # [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] | ||
| # helix-tui = { path = "../helix-tui" } |
There was a problem hiding this comment.
I think all our cargo.tomls have newlines at the end
Suggested change
| # helix-tui = { path = "../helix-tui" } | |
| # helix-tui = { path = "../helix-tui" } | |
| @@ -28,7 +28,7 @@ path = "src/main.rs" | |||
|
|
|||
| [dependencies] | |||
| helix-core = { version = "0.6", path = "../helix-core" } | |||
| helix-view = { version = "0.6", path = "../helix-view" } | |||
| helix-view = { version = "0.6", path = "../helix-view", features = ["term"]} | |||
There was a problem hiding this comment.
Suggested change
| helix-view = { version = "0.6", path = "../helix-view", features = ["term"]} | |
| helix-view = { version = "0.6", path = "../helix-view", features = ["term"] } |
gabydd
reviewed
Aug 4, 2023
|
I think I want to go with a slightly different approach here, this branch also has an insane amount of conflicts so I need to start over anyway. I will close this |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5555
This PR is super simple. All it does is:
helix-viewhelix_term::withcrate::helix_view::withcrate::(unfortunate that rust doesn't allow this)tui::withhelix_tui::(seems more consistent, but the main reason is that RA for some reason is unable to resolve these imports evenotugh rustc can which leads to terrible dev experience)helix-termthat are used by commandsThe primary focus of this PR is to avoid git conflicts. Git should handle most of these changes fine if a PR is rebased
altough there can be some conflicts in the
usestatements. However, compared to other PRs the changes here are notthat large and should be easy to resolve.
These changes will not only allow experimentation with GUIs (#39) but also unblocks other efforts (see #5555).
However, this PR implements not abstraction over the actual terminal functionality yet.
I think the best approach would be a
cfgbased approach opposed to traits as used by @archser in theguibranch.Such an approach avoids many breaking changes (and making every function/struct generic).
This abstraction can be added in the future with smaller less disruptive PRs.