From d568d9f5d0c36728afad1aaf365e91a9d3e0b899 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 10 Aug 2023 15:26:54 +0200 Subject: [PATCH] Lint vertical spacing in the code (#3224) * Lint vertical spacing in the code * Add some vertical spacing for readability --- .github/workflows/rust.yml | 7 +- CONTRIBUTING.md | 2 +- crates/eframe/src/lib.rs | 1 + crates/eframe/src/native/epi_integration.rs | 2 + crates/eframe/src/native/run.rs | 1 + crates/egui/src/containers/scroll_area.rs | 6 + crates/egui/src/data/input.rs | 10 ++ crates/egui/src/grid.rs | 1 + crates/egui/src/memory.rs | 2 + crates/egui/src/widgets/button.rs | 1 + crates/egui/src/widgets/drag_value.rs | 1 + crates/egui/src/widgets/plot/items/mod.rs | 11 ++ crates/egui/src/widgets/plot/mod.rs | 2 + crates/egui/src/widgets/slider.rs | 2 + crates/egui_extras/src/image.rs | 4 + crates/egui_extras/src/layout.rs | 2 + crates/egui_extras/src/table.rs | 6 + crates/epaint/src/tessellator.rs | 20 ++- crates/epaint/src/text/font.rs | 6 + crates/epaint/src/text/text_layout_types.rs | 8 ++ crates/epaint/src/textures.rs | 2 + scripts/lint.py | 132 ++++++++++++++++++++ 22 files changed, 215 insertions(+), 14 deletions(-) create mode 100755 scripts/lint.py diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 55865370..6d4e8e5d 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -1,6 +1,6 @@ on: [push, pull_request] -name: CI +name: Rust env: # web_sys_unstable_apis is required to enable the web_sys clipboard API which eframe web uses, @@ -37,6 +37,9 @@ jobs: - name: Rustfmt run: cargo fmt --all -- --check + - name: Lint vertical spacing + run: ./scripts/lint.py + - name: Install cargo-cranky uses: baptiste0928/cargo-install@v1 with: @@ -145,7 +148,7 @@ jobs: rust-version: "1.65.0" log-level: error command: check - arguments: ${{ matrix.flags }} --target ${{ matrix.target }} + arguments: --target ${{ matrix.target }} # --------------------------------------------------------------------------- diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 28ff70d4..8d389745 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -60,7 +60,7 @@ Read the section on integrations at ## Code Conventions Conventions unless otherwise specified: -* angles are in radians +* angles are in radians and clock-wise * `Vec2::X` is right and `Vec2::Y` is down. * `Pos2::ZERO` is left top. diff --git a/crates/eframe/src/lib.rs b/crates/eframe/src/lib.rs index 8164e7e6..8093339e 100644 --- a/crates/eframe/src/lib.rs +++ b/crates/eframe/src/lib.rs @@ -272,6 +272,7 @@ pub fn run_simple_native( struct SimpleApp { update_fun: U, } + impl App for SimpleApp { fn update(&mut self, ctx: &egui::Context, frame: &mut Frame) { (self.update_fun)(ctx, frame); diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index faed7360..a0bec23b 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -335,8 +335,10 @@ pub struct EpiIntegration { pub egui_ctx: egui::Context, pending_full_output: egui::FullOutput, egui_winit: egui_winit::State, + /// When set, it is time to close the native window. close: bool, + can_drag_window: bool, window_state: WindowState, follow_system_theme: bool, diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index 3b4cc5c4..3672c9bf 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -22,6 +22,7 @@ use super::epi_integration::{self, EpiIntegration}; pub enum UserEvent { RequestRepaint { when: Instant, + /// What the frame number was when the repaint was _requested_. frame_nr: u64, }, diff --git a/crates/egui/src/containers/scroll_area.rs b/crates/egui/src/containers/scroll_area.rs index b75a16ab..8bd8142c 100644 --- a/crates/egui/src/containers/scroll_area.rs +++ b/crates/egui/src/containers/scroll_area.rs @@ -334,16 +334,22 @@ struct Prepared { state: State, has_bar: [bool; 2], auto_shrink: [bool; 2], + /// How much horizontal and vertical space are used up by the /// width of the vertical bar, and the height of the horizontal bar? current_bar_use: Vec2, + scroll_bar_visibility: ScrollBarVisibility, + /// Where on the screen the content is (excludes scroll bars). inner_rect: Rect, + content_ui: Ui, + /// Relative coordinates: the offset and size of the view of the inner UI. /// `viewport.min == ZERO` means we scrolled to the top. viewport: Rect, + scrolling_enabled: bool, stick_to_end: [bool; 2], } diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index 29ff3f5d..4bb9fdc0 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -693,27 +693,37 @@ pub enum Key { /// The virtual keycode for the Minus key. Minus, + /// The virtual keycode for the Plus/Equals key. PlusEquals, /// Either from the main row or from the numpad. Num0, + /// Either from the main row or from the numpad. Num1, + /// Either from the main row or from the numpad. Num2, + /// Either from the main row or from the numpad. Num3, + /// Either from the main row or from the numpad. Num4, + /// Either from the main row or from the numpad. Num5, + /// Either from the main row or from the numpad. Num6, + /// Either from the main row or from the numpad. Num7, + /// Either from the main row or from the numpad. Num8, + /// Either from the main row or from the numpad. Num9, diff --git a/crates/egui/src/grid.rs b/crates/egui/src/grid.rs index 314266a7..eb33e00e 100644 --- a/crates/egui/src/grid.rs +++ b/crates/egui/src/grid.rs @@ -60,6 +60,7 @@ pub(crate) struct GridLayout { /// State previous frame (if any). /// This can be used to predict future sizes of cells. prev_state: State, + /// State accumulated during the current frame. curr_state: State, initial_available: Rect, diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs index db76f7e7..d89fd344 100644 --- a/crates/egui/src/memory.rs +++ b/crates/egui/src/memory.rs @@ -546,8 +546,10 @@ impl Memory { #[cfg_attr(feature = "serde", serde(default))] pub struct Areas { areas: IdMap, + /// Back-to-front. Top is last. order: Vec, + visible_last_frame: ahash::HashSet, visible_current_frame: ahash::HashSet, diff --git a/crates/egui/src/widgets/button.rs b/crates/egui/src/widgets/button.rs index a7fa6781..ca211987 100644 --- a/crates/egui/src/widgets/button.rs +++ b/crates/egui/src/widgets/button.rs @@ -23,6 +23,7 @@ pub struct Button { text: WidgetText, shortcut_text: WidgetText, wrap: Option, + /// None means default for interact fill: Option, stroke: Option, diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 1a965d4e..b09033c1 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -11,6 +11,7 @@ use crate::*; pub(crate) struct MonoState { last_dragged_id: Option, last_dragged_value: Option, + /// For temporary edit of a [`DragValue`] value. /// Couples with the current focus id. edit_string: Option, diff --git a/crates/egui/src/widgets/plot/items/mod.rs b/crates/egui/src/widgets/plot/items/mod.rs index 3f21b0e8..f18f34af 100644 --- a/crates/egui/src/widgets/plot/items/mod.rs +++ b/crates/egui/src/widgets/plot/items/mod.rs @@ -760,15 +760,22 @@ impl PlotItem for Text { /// A set of points. pub struct Points { pub(super) series: PlotPoints, + pub(super) shape: MarkerShape, + /// Color of the marker. `Color32::TRANSPARENT` means that it will be picked automatically. pub(super) color: Color32, + /// Whether to fill the marker. Does not apply to all types. pub(super) filled: bool, + /// The maximum extent of the marker from its center. pub(super) radius: f32, + pub(super) name: String, + pub(super) highlight: bool, + pub(super) stems: Option, } @@ -1290,8 +1297,10 @@ pub struct BarChart { pub(super) bars: Vec, pub(super) default_color: Color32, pub(super) name: String, + /// A custom element formatter pub(super) element_formatter: Option String>>, + highlight: bool, } @@ -1460,8 +1469,10 @@ pub struct BoxPlot { pub(super) boxes: Vec, pub(super) default_color: Color32, pub(super) name: String, + /// A custom element formatter pub(super) element_formatter: Option String>>, + highlight: bool, } diff --git a/crates/egui/src/widgets/plot/mod.rs b/crates/egui/src/widgets/plot/mod.rs index 9fa09b7e..9eca796d 100644 --- a/crates/egui/src/widgets/plot/mod.rs +++ b/crates/egui/src/widgets/plot/mod.rs @@ -101,9 +101,11 @@ struct PlotMemory { /// Indicates if the user has modified the bounds, for example by moving or zooming, /// or if the bounds should be calculated based by included point or auto bounds. bounds_modified: AxisBools, + hovered_entry: Option, hidden_items: ahash::HashSet, last_plot_transform: PlotTransform, + /// Allows to remember the first click position when performing a boxed zoom last_click_pos_for_zoom: Option, } diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index f298a783..f793d86e 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -77,8 +77,10 @@ pub struct Slider<'a> { prefix: String, suffix: String, text: WidgetText, + /// Sets the minimal step of the widget value step: Option, + drag_value_speed: Option, min_decimals: usize, max_decimals: Option, diff --git a/crates/egui_extras/src/image.rs b/crates/egui_extras/src/image.rs index 3526b6de..ba8acaf4 100644 --- a/crates/egui_extras/src/image.rs +++ b/crates/egui_extras/src/image.rs @@ -10,11 +10,15 @@ pub use usvg::FitTo; /// Use the `svg` and `image` features to enable more constructors. pub struct RetainedImage { debug_name: String, + size: [usize; 2], + /// Cleared once [`Self::texture`] has been loaded. image: Mutex, + /// Lazily loaded when we have an egui context. texture: Mutex>, + options: TextureOptions, } diff --git a/crates/egui_extras/src/layout.rs b/crates/egui_extras/src/layout.rs index 932594bf..fbd15e7a 100644 --- a/crates/egui_extras/src/layout.rs +++ b/crates/egui_extras/src/layout.rs @@ -32,9 +32,11 @@ pub struct StripLayout<'l> { direction: CellDirection, pub(crate) rect: Rect, pub(crate) cursor: Pos2, + /// Keeps track of the max used position, /// so we know how much space we used. max: Pos2, + cell_layout: egui::Layout, } diff --git a/crates/egui_extras/src/table.rs b/crates/egui_extras/src/table.rs index 9966a8b9..de0e9870 100644 --- a/crates/egui_extras/src/table.rs +++ b/crates/egui_extras/src/table.rs @@ -28,7 +28,9 @@ enum InitialColumnSize { #[derive(Clone, Copy, Debug, PartialEq)] pub struct Column { initial_width: InitialColumnSize, + width_range: Rangef, + /// Clip contents if too narrow? clip: bool, @@ -511,8 +513,10 @@ pub struct Table<'a> { columns: Vec, available_width: f32, state: TableState, + /// Accumulated maximum used widths for each column. max_used_widths: Vec, + first_frame_auto_size_columns: bool, resizable: bool, striped: bool, @@ -1011,8 +1015,10 @@ pub struct TableRow<'a, 'b> { layout: &'b mut StripLayout<'a>, columns: &'b [Column], widths: &'b [f32], + /// grows during building with the maximum widths max_used_widths: &'b mut [f32], + col_index: usize, striped: bool, height: f32, diff --git a/crates/epaint/src/tessellator.rs b/crates/epaint/src/tessellator.rs index 6671859b..facb2612 100644 --- a/crates/epaint/src/tessellator.rs +++ b/crates/epaint/src/tessellator.rs @@ -13,17 +13,15 @@ use emath::*; #[allow(clippy::approx_constant)] mod precomputed_vertices { - /* - fn main() { - let n = 64; - println!("pub const CIRCLE_{}: [Vec2; {}] = [", n, n+1); - for i in 0..=n { - let a = std::f64::consts::TAU * i as f64 / n as f64; - println!(" vec2({:.06}, {:.06}),", a.cos(), a.sin()); - } - println!("];") - } - */ + // fn main() { + // let n = 64; + // println!("pub const CIRCLE_{}: [Vec2; {}] = [", n, n+1); + // for i in 0..=n { + // let a = std::f64::consts::TAU * i as f64 / n as f64; + // println!(" vec2({:.06}, {:.06}),", a.cos(), a.sin()); + // } + // println!("];") + // } use emath::{vec2, Vec2}; diff --git a/crates/epaint/src/text/font.rs b/crates/epaint/src/text/font.rs index 939cdf9e..4cf26014 100644 --- a/crates/epaint/src/text/font.rs +++ b/crates/epaint/src/text/font.rs @@ -78,11 +78,15 @@ impl Default for GlyphInfo { pub struct FontImpl { name: String, ab_glyph_font: ab_glyph::FontArc, + /// Maximum character height scale_in_pixels: u32, + height_in_points: f32, + // move each character by this much (hack) y_offset: f32, + ascent: f32, pixels_per_point: f32, glyph_info_cache: RwLock>, // TODO(emilk): standard Mutex @@ -320,8 +324,10 @@ type FontIndex = usize; /// Wrapper over multiple [`FontImpl`] (e.g. a primary + fallbacks for emojis) pub struct Font { fonts: Vec>, + /// Lazily calculated. characters: Option>, + replacement_glyph: (FontIndex, GlyphInfo), pixels_per_point: f32, row_height: f32, diff --git a/crates/epaint/src/text/text_layout_types.rs b/crates/epaint/src/text/text_layout_types.rs index 28301c85..8ec829b8 100644 --- a/crates/epaint/src/text/text_layout_types.rs +++ b/crates/epaint/src/text/text_layout_types.rs @@ -193,8 +193,10 @@ impl std::hash::Hash for LayoutJob { pub struct LayoutSection { /// Can be used for first row indentation. pub leading_space: f32, + /// Range into the galley text pub byte_range: Range, + pub format: TextFormat, } @@ -218,12 +220,18 @@ impl std::hash::Hash for LayoutSection { #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct TextFormat { pub font_id: FontId, + /// Text color pub color: Color32, + pub background: Color32, + pub italics: bool, + pub underline: Stroke, + pub strikethrough: Stroke, + /// If you use a small font and [`Align::TOP`] you /// can get the effect of raised text. pub valign: Align, diff --git a/crates/epaint/src/textures.rs b/crates/epaint/src/textures.rs index e6b42374..abe8d0b9 100644 --- a/crates/epaint/src/textures.rs +++ b/crates/epaint/src/textures.rs @@ -9,8 +9,10 @@ use crate::{ImageData, ImageDelta, TextureId}; pub struct TextureManager { /// We allocate texture id:s linearly. next_id: u64, + /// Information about currently allocated textures. metas: ahash::HashMap, + delta: TexturesDelta, } diff --git a/scripts/lint.py b/scripts/lint.py new file mode 100755 index 00000000..93851ffa --- /dev/null +++ b/scripts/lint.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python +""" +Runs custom linting on Rust code. +""" + +import argparse +import os +import re +import sys + + +def lint_file_path(filepath, args) -> int: + with open(filepath) as f: + lines_in = f.readlines() + + errors, lines_out = lint_lines(filepath, lines_in) + + for error in errors: + print(error) + + if args.fix and lines_in != lines_out: + with open(filepath, 'w') as f: + f.writelines(lines_out) + print(f'{filepath} fixed.') + + return len(errors) + + + +def lint_lines(filepath, lines_in): + last_line_was_empty = True + + errors = [] + lines_out = [] + + for line_nr, line in enumerate(lines_in): + line_nr = line_nr+1 + + # TODO: only # and /// on lines before a keyword + + pattern = r'^\s*((///)|((pub(\(\w*\))? )?((impl|fn|struct|enum|union|trait)\b))).*$' + if re.match(pattern, line): + if not last_line_was_empty: + errors.append( + f'{filepath}:{line_nr}: for readability, add newline before `{line.strip()}`') + lines_out.append("\n") + lines_out.append(line) + + stripped = line.strip() + last_line_was_empty = stripped == '' or \ + stripped.startswith('#') or \ + stripped.startswith('//') or \ + stripped.endswith('{') or \ + stripped.endswith('(') or \ + stripped.endswith('\\') or \ + stripped.endswith('r"') or \ + stripped.endswith(']') + + return errors, lines_out + + +def test_lint(): + should_pass = [ + "hello world", + """ + /// docstring + foo + + /// docstring + bar + """ + ] + + should_fail = [ + """ + /// docstring + foo + /// docstring + bar + """ + ] + + for code in should_pass: + errors, _ = lint_lines("test.py", code.split('\n')) + assert len(errors) == 0, f'expected this to pass:\n{code}\ngot: {errors}' + + for code in should_fail: + errors, _ = lint_lines("test.py", code.split('\n')) + assert len(errors) > 0, f'expected this to fail:\n{code}' + + pass + + +def main(): + test_lint() # Make sure we are bug free before we run! + + parser = argparse.ArgumentParser( + description='Lint Rust code with custom linter.') + parser.add_argument('files', metavar='file', type=str, nargs='*', + help='File paths. Empty = all files, recursively.') + parser.add_argument('--fix', dest='fix', action='store_true', + help='Automatically fix the files') + + args = parser.parse_args() + + num_errors = 0 + + if args.files: + for filepath in args.files: + num_errors += lint_file_path(filepath, args) + else: + script_dirpath = os.path.dirname(os.path.realpath(__file__)) + root_dirpath = os.path.abspath(f'{script_dirpath}/..') + os.chdir(root_dirpath) + + exclude = set(['target', 'target_ra']) + for root, dirs, files in os.walk('.', topdown=True): + dirs[:] = [d for d in dirs if d not in exclude] + for filename in files: + if filename.endswith('.rs'): + filepath = os.path.join(root, filename) + num_errors += lint_file_path(filepath, args) + + if num_errors == 0: + print(f"{sys.argv[0]} finished without error") + sys.exit(0) + else: + print(f"{sys.argv[0]} found {num_errors} errors.") + sys.exit(1) + +if __name__ == '__main__': + main()