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.

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

Remove dependency on ustring #92

Closed
migueldeicaza opened this issue May 18, 2018 · 7 comments
Closed

Remove dependency on ustring #92

migueldeicaza opened this issue May 18, 2018 · 7 comments
Labels
breaking-change For PRs that introduces a breaking change (behavior or API) design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) v2 For discussions, issues, etc... relavant for v2

Comments

@migueldeicaza
Copy link
Collaborator

We might need to introduce string overloads

@jpierson
Copy link

I just ran into this myself. My gut was to look for a constructor on ustring that takes a string literal now I'm looking for the most reasonable short term solution.

@jpierson
Copy link

To help explain to others who may not be fully in on what makes this annoying in F# is to look at an example case in F# from the Sample in the README where we are instantiating an instance of the Terminal.Gui.Window class.

var win = new Window (new Rect (0, 1, top.Frame.Width, top.Frame.Height-1), "MyApp");
let win = new Window(new Rect(0, 1, top.Frame.Width, top.Frame.Height - 1), NStack.ustring.op_Implicit "MyApp")

Notice the need in the F# example to call the underlying compiled name for the implicit string to ustring operator op_Implicit. Not the end of the world but having this bit of the code read something like ustring "MyApp" without having to introduce any helper functions would make this library feel a bit more natural in F#.

@ebekker
Copy link

ebekker commented Aug 22, 2018

Isn't the more functional approach in F# to pipe the value, such as "foobar" |> ustring?

@jpierson
Copy link

jpierson commented Aug 23, 2018

@ebekker, that definitely looks valid as long as ustring is not a constructor but whether it's more functional or not I think is subjective. I typically use the pipe forward operator for larger chains of transformations.

In this case since there are no additional parenthesis for needed for the prefix notation I think it comes down to which one is less characters and thus noise.

"foobar" |> ustring

v.s.

ustring "foobar"

@migueldeicaza
Copy link
Collaborator Author

Yeah, I think I might need to make ustring just an internal API, and an optional API everywhere :-(

@asik
Copy link

asik commented Apr 29, 2019

You run into the same issue using SharpDX. That's why I always define an implicit conversion operator:

let inline (!>) (x:^a) : ^b = ((^a or ^b) : (static member op_Implicit : ^a -> ^b) x)

Then you just write !> "blabla" and don't need to worry what the target type is.
Example usage here:
https://gist.github.com/asik/ce229023fb0eb153bde6aacc384d8ac4

Agreed it would be better to avoid needing this, but just to say there's a half-decent workaround.

@tig tig added the design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) label May 24, 2020
@tig tig changed the title F# the ustring API makes it annoying to use F#. Remove dependency on ustring Oct 14, 2022
@tig
Copy link
Collaborator

tig commented Oct 14, 2022

Changed title of this Issue from "F# the ustring API makes it annoying to use F#".

In v2.0 we should remove the dependency on ustring (at least externally) and use System.Rune instead. While ustring is neat, the fact that artifacts of it are exposed via the public Terminal.Gui API is problematic (F# support just being one). Another example is just simple usage of the API requires users to import NStack (see the C# Example in the main README).

@tig tig added this to the v2.0 milestone Oct 14, 2022
@tig tig removed this from the v2.0 milestone Feb 28, 2023
@tig tig added the breaking-change For PRs that introduces a breaking change (behavior or API) label Mar 26, 2023
@tig tig added the v2 For discussions, issues, etc... relavant for v2 label Apr 19, 2023
tig added a commit that referenced this issue May 20, 2023
* Remove NStack and replace ustring to string.

* Add unit test and improving some code.

* Adjust code and fix all unit tests errors.

* Add XML Document and move the Rune folder into the Text folder.

* Improve unit tests with byte array on DecodeRune and DecodeLastRune.

* Fix unit test.

* 😂Code review

* Reduce unit tests code.

* Change StringExtensions.Make to StringExtensions.ToString and added some more unit tests.

* Fix merge errors.

* Remove GetTextWidth and calls replaced with StringExtensions.GetColumns.

* Hack to use UseSystemConsole passed in the command line arguments.

* Revert "Hack to use UseSystemConsole passed in the command line arguments."

This reverts commit b74d11c.

* Remove Application.UseSystemConsole from the config file.

* Fix errors related by removing UseSystemConsole from the config file.

* Fixes #2633. DecodeEscSeq throw an exception if cki is null.

* Fix an exception if SelectedItem is -1.

* Set SelectedItem to 0 and remove unnecessary ToString.

* Using a unique ToString method for Rune and other for byte.

* Fix a bug where a wider rune is added with only a width of 1.

* Force the SelectedGlyph is the one that was typed after jumpList is executed.

* Added more InlineData to RuneTests.

* Reducing significantly the code by using Theory attribute in the TextFormatterTests.

* Override PositionCursor to handle the CharMap cursor position.

* Fix merge errors.

* Minor tweaks to API docs

---------

Co-authored-by: Tig Kindel <tig@users.noreply.github.com>
@tig tig closed this as completed May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change For PRs that introduces a breaking change (behavior or API) design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) v2 For discussions, issues, etc... relavant for v2
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

5 participants