deprecate ShowContextual, document changes to colors/color_new modules

6 files changed, 59 insertions, 22 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 502b165..3178110 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -40,6 +40,27 @@ made VecSink's `records` private. instead of extracting records from the struct
 
 made VecSink is now available through the `alloc` feature flag as well as `std`.
 
+significantly reduced `mod colored`:
+  generally, colorization of text is a presentation issue; `trait Colorize`
+  mixed formatting of data to text with how that text is presented, but that is
+  at odds with the same text being presented in different ways for which
+  colorization is not generic. for example, rendering an instruction as marked
+  up HTML involves coloring in an entirely different way than rendering an
+  instruction with ANSI sequences for a VT100-like terminal.
+
+  the changes to `mod colored` reflect this, and rely on impls of `DisplaySink`
+  to know what they are displaying into and what is the correct mechanism to
+  present styled text.
+
+  with this in mind:
+  * the module `yaxpeax_arch::color` now only exists with the `colors` feature
+  * removed `color::Colorize`, `color::Colored`
+  * `enum Colored` is closest in spirit to `enum Color`
+  * `trait YaxColors` no longer takes displayable types in its functions, no longer returns displayable types.
+  * `trait YaxColors`' sole function is as an inlining-friendly settings
+    mechanism for `DisplaySink` impls that care about coloring output
+  * dropped dependency on `crossterm`
+
 ## 0.2.8
 
 added an impl of `From<ReadError>` for `StandardPartialDecoderError`, matching the existing `StandardDecodeError` impl.
diff --git a/Cargo.toml b/Cargo.toml
index e1d3357..5dba621 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -23,7 +23,7 @@ thiserror = "1.0.26"
 lto = true
 
 [features]
-default = ["std", "alloc", "use-serde", "colors", "address-parse"]
+default = ["std", "alloc", "use-serde", "color-new", "address-parse"]
 
 std = ["alloc"]
 
@@ -33,6 +33,14 @@ alloc = []
 # Arch and Arch::Address
 use-serde = ["serde", "serde_derive"]
 
+# feature flag for the existing but misfeature'd initial support for output
+# coloring.  the module this gates will be removed in 0.4.0, which includes
+# removing `trait Colorize`, and requires a major version bump for any
+# dependency that moves forward.
 colors = ["crossterm"]
 
+# feature flag for revised output colorizing support, which will replace the
+# existing `colors` feature in 0.4.0.
+color-new = []
+
 address-parse = []
diff --git a/Makefile b/Makefile
index aa38b87..efe5c3f 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-test: test-std test-no-std test-serde-no-std test-alloc-no-std
+test: test-std test-no-std test-serde-no-std test-colors-no-std test-color-new-no-std test-alloc-no-std
 
 test-std:
 	cargo test
@@ -6,5 +6,9 @@ test-no-std:
 	cargo test --no-default-features
 test-serde-no-std:
 	cargo test --no-default-features --features "serde"
+test-colors-no-std:
+	cargo test --no-default-features --features "colors"
+test-color-new-no-std:
+	cargo test --no-default-features --features "color-new"
 test-alloc-no-std:
 	cargo test --no-default-features --features "alloc"
diff --git a/goodfile b/goodfile
index 5264b18..97b5ffa 100644
--- a/goodfile
+++ b/goodfile
@@ -17,6 +17,7 @@ Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "c
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "use-serde"}, {name="test feature combinations"})
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "address-parse"}, {name="test feature combinations"})
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "alloc"}, {name="test feature combinations"})
+Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "color-new"}, {name="test feature combinations"})
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "std,colors"}, {name="test feature combinations"})
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "std,use-serde"}, {name="test feature combinations"})
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "std,address-parse"}, {name="test feature combinations"})
@@ -27,3 +28,4 @@ Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "s
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "std,colors,address-parse,alloc"}, {name="test feature combinations"})
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "std,use-serde,colors"}, {name="test feature combinations"})
 Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "std,use-serde,colors,alloc"}, {name="test feature combinations"})
+Build.run({"cargo", "test", "--no-default-features", "--tests", "--features", "color-new,alloc"}, {name="test feature combinations"})
diff --git a/src/display.rs b/src/display.rs
index 3965bdc..754d3e6 100644
--- a/src/display.rs
+++ b/src/display.rs
@@ -44,36 +44,36 @@ pub enum NumberStyleHint {
 }
 
 #[deprecated(since="0.3.0", note="format_number_i32 is both slow and incorrect: YaxColors may not result in correct styling when writing anywhere other than a terminal, and both stylin and formatting does not inline as well as initially expected. see DisplaySink instead.")]
-pub fn format_number_i32<W: fmt::Write, Y: YaxColors>(colors: &Y, f: &mut W, i: i32, hint: NumberStyleHint) -> fmt::Result {
+pub fn format_number_i32<W: fmt::Write, Y: YaxColors>(_colors: &Y, f: &mut W, i: i32, hint: NumberStyleHint) -> fmt::Result {
     match hint {
         NumberStyleHint::Signed => {
-            write!(f, "{}", colors.number(i))
+            write!(f, "{}", (i))
         },
         NumberStyleHint::HexSigned => {
-            write!(f, "{}", colors.number(signed_i32_hex(i)))
+            write!(f, "{}", signed_i32_hex(i))
         },
         NumberStyleHint::Unsigned => {
-            write!(f, "{}", colors.number(i as u32))
+            write!(f, "{}", i as u32)
         },
         NumberStyleHint::HexUnsigned => {
-            write!(f, "{}", colors.number(u32_hex(i as u32)))
+            write!(f, "{}", u32_hex(i as u32))
         },
         NumberStyleHint::SignedWithSignSplit => {
             if i == core::i32::MIN {
-                write!(f, "- {}", colors.number("2147483647"))
+                write!(f, "- {}", "2147483647")
             } else if i < 0 {
-                write!(f, "- {}", colors.number(-Wrapping(i)))
+                write!(f, "- {}", -Wrapping(i))
             } else {
-                write!(f, "+ {}", colors.number(i))
+                write!(f, "+ {}", i)
             }
         }
         NumberStyleHint::HexSignedWithSignSplit => {
             if i == core::i32::MIN {
-                write!(f, "- {}", colors.number("0x7fffffff"))
+                write!(f, "- {}", ("0x7fffffff"))
             } else if i < 0 {
-                write!(f, "- {}", colors.number(u32_hex((-Wrapping(i)).0 as u32)))
+                write!(f, "- {}", u32_hex((-Wrapping(i)).0 as u32))
             } else {
-                write!(f, "+ {}", colors.number(u32_hex(i as u32)))
+                write!(f, "+ {}", u32_hex(i as u32))
             }
         },
         NumberStyleHint::HexSignedWithSign => {
diff --git a/src/lib.rs b/src/lib.rs
index 3af5f00..f5b6ea6 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,12 +1,14 @@
 #![no_std]
 #![doc = include_str!("../README.md")]
 
+#[cfg(feature = "alloc")]
+extern crate alloc;
+
 use core::fmt::{self, Debug, Display};
 use core::hash::Hash;
 
 #[cfg(feature="use-serde")]
 #[macro_use] extern crate serde_derive;
-
 #[cfg(feature="use-serde")]
 use serde::{Serialize, Deserialize};
 
@@ -18,24 +20,22 @@ pub use address::AddrParse;
 
 pub mod annotation;
 
+#[deprecated(since="0.3.0", note="yaxpeax_arch::color conflates output mechanism and styling, leaving it brittle and overly-restrictive. see `yaxpeax_arch::color_new`, which will replace `color` in a future version.")]
 mod color;
+#[allow(deprecated)] // allow exporting the deprecated items here to not break downstreams even further...
 pub use color::{Colorize, NoColors, YaxColors};
-
-#[cfg(feature="colors")]
-pub use color::ColorSettings;
-
-#[cfg(feature = "alloc")]
-extern crate alloc;
+#[cfg(feature="color-new")]
+pub mod color_new;
 
 pub mod display;
 
-pub mod testkit;
-
 mod reader;
 pub use reader::{Reader, ReaderBuilder, ReadError, U8Reader, U16le, U16be, U32le, U32be, U64le, U64be};
 
 pub mod safer_unchecked;
 
+pub mod testkit;
+
 /// the minimum set of errors a `yaxpeax-arch` disassembler may produce.
 ///
 /// it is permissible for an implementer of `DecodeError` to have items that return `false` for
@@ -235,6 +235,8 @@ pub trait Instruction {
     fn well_defined(&self) -> bool;
 }
 
+#[allow(deprecated)]
+#[deprecated(since="0.3.0", note="ShowContextual ties YaxColors and fmt::Write in a way that only sometimes composes. simultaneously, it is too generic on Ctx, making it difficult to implement and use. it will be revisited in the future.")]
 pub trait ShowContextual<Addr, Ctx: ?Sized, T: fmt::Write, Y: YaxColors> {
     fn contextualize(&self, colors: &Y, address: Addr, context: Option<&Ctx>, out: &mut T) -> fmt::Result;
 }