ronantakizawa/codereview-bench huggingface dataset:
Repository: veler/DevToys
File: src/dev/impl/DevToys/ViewModels/Tools/Text/StringUtilities/StringUtilitiesToolViewModel.cs (C#)

Code before:
            lock (_lockObject)
            {
                _forbidBackup = true;
                Text = new string(titleCaseString);
                _forbidBackup = false;
            }
        }

        #endregion

        #region AlphabetizeCommand

        public IRelayCommand AlphabetizeCommand { get; }

        private void ExecuteAlphabetizeCommand()
        {
            string? text = _textBackup;
            if (text is null)
            {
                return;
            }

            IOrderedEnumerable<string> lines = text.Split('\r').OrderBy(line => line, StringComparer.CurrentCulture);
            text = string.Join('\r', lines);

            lock (_lockObject)
            {
                _forbidBackup = true;
                Text = text;
                _forbidBackup = false;
            }
        }

        #endregion

        #region ReverseAlphabetizeCommand

        public IRelayCommand ReverseAlphabetizeCommand { get; }

        private void ExecuteReverseAlphabetizeCommand()
        {
            string? text = _textBackup;
            if (text is null)
            {
                return;
            }

            IOrderedEnumerable<string> lines = text.Split('\r').OrderByDescending(line => line, StringComparer.CurrentCulture);
            text = string.Join('\r', lines);

            lock (_lockObject)

Review comment (refactor):
We do this block of code at many places, including in the older code I wrote originally (oops). Could we create a helper method `SetTextWithoutBackup` that ultimately runs this block?

Code after:

            SetTextWithoutBackup(text);
        }

        #endregion

        #region ReverseAlphabetizeLastCommand

        public IRelayCommand ReverseAlphabetizeLastCommand { get; }

        private void ExecuteReverseAlphabetizeLastCommand()
        {
            string? text = _textBackup;
            if (text is null)
            {
                return;
            }

            var lines = text.Split('\r').ToList();
            lines.Sort((line1, line2) =>
            {
                string line1LastWord = new(line1.Reverse().TakeWhile(char.IsLetterOrDigit).Reverse().ToArray());
                string line2LastWord = new(line2.Reverse().TakeWhile(char.IsLetterOrDigit).Reverse().ToArray());

                return string.Compare(line1LastWord, line2LastWord, StringComparison.CurrentCulture) * -1;
            });
            text = string.Join('\r', lines);

            SetTextWithoutBackup(text);
        }

        #endregion

        #region ReverseCommand

        public IRelayCommand ReverseCommand { get; }

        private void ExecuteReverseCommand()
        {
            string? text = _textBackup;
            if (text is null)
            {
                return;
            }

            IEnumerable<string> lines = text.Split('\r').Reverse();
            text = string.Join('\r', lines);

            SetTextWithoutBackup(text);
        }
Repository: veler/DevToys
File: src/dev/impl/DevToys/ViewModels/Tools/Text/StringUtilities/StringUtilitiesToolViewModel.cs (C#)

Code before:
            }

            bool lowerCase = true;
            for (int i = 0; i < titleCaseString.Length; i++)
            {
                if (lowerCase)
                {
                    titleCaseString[i] = char.ToLowerInvariant(titleCaseString[i]);
                }
                else
                {
                    titleCaseString[i] = char.ToUpperInvariant(titleCaseString[i]);
                }

                lowerCase = !lowerCase;
            }

            lock (_lockObject)
            {
                _forbidBackup = true;
                Text = new string(titleCaseString);
                _forbidBackup = false;
            }
        }

        #endregion

        #region InverseCaseCommand

        public IRelayCommand InverseCaseCommand { get; }

        private void ExecuteInverseCaseCommand()
        {
            char[]? titleCaseString = _textBackup?.ToCharArray();
            if (titleCaseString is null)
            {
                return;
            }

            bool lowerCase = false;
            for (int i = 0; i < titleCaseString.Length; i++)
            {
                if (lowerCase)
                {
                    titleCaseString[i] = char.ToLowerInvariant(titleCaseString[i]);
                }
                else
                {
                    titleCaseString[i] = char.ToUpperInvariant(titleCaseString[i]);
                }


Review comment (suggestion):
Could we use `Environment.NewLine` instead of `\r` ? It's more future-proof as DevToys 2.0 will target MacOS and Linux.

Code after:
            if (text is null)
            {
                return;
            }

            IOrderedEnumerable<string> lines = text.Split('\r').OrderBy(line => line, StringComparer.CurrentCulture);
            text = string.Join('\r', lines);

            SetTextWithoutBackup(text);
        }

        #endregion

        #region ReverseAlphabetizeCommand

        public IRelayCommand ReverseAlphabetizeCommand { get; }

        private void ExecuteReverseAlphabetizeCommand()
        {
            string? text = _textBackup;
            if (text is null)
            {
                return;
            }

            IOrderedEnumerable<string> lines = text.Split('\r').OrderByDescending(line => line, StringComparer.CurrentCulture);
            text = string.Join('\r', lines);

            SetTextWithoutBackup(text);
        }

        #endregion

        #region AlphabetizeLastCommand

        public IRelayCommand AlphabetizeLastCommand { get; }

        private void ExecuteAlphabetizeLastCommand()
        {
            string? text = _textBackup;
            if (text is null)
            {
                return;
            }

            var lines = text.Split('\r').ToList();
            lines.Sort((line1, line2) =>
            {
                string line1LastWord = new(line1.Reverse().TakeWhile(char.IsLetterOrDigit).Reverse().ToArray());
                string line2LastWord = new(line2.Reverse().TakeWhile(char.IsLetterOrDigit).Reverse().ToArray());
Repository: veler/DevToys
File: src/app/dev/platforms/desktop/DevToys.Linux/Core/ThemeListener.cs (C#)

Code before:
        }
    }

    private bool GetBestValueForCompactMode()
    {
        if (UserIsCompactModePreference)
        {
            return true;
        }

        if (_mainWindow is not null)
        {
            return _mainWindow.GetWidth() <= NavBarThresholds.NavBarWidthSidebarCollapseThreshold;
        }

        return false;
    }

    private void UpdateSystemSettingsAndApplyTheme()
    {
        IsHighContrast = _adwStyleManager.HighContrast;
        CurrentSystemTheme = GetCurrentSystemTheme();

        ApplyDesiredColorTheme();
    }

    private AvailableApplicationTheme GetCurrentSystemTheme()
    {
        return _gtkSettings.GtkApplicationPreferDarkTheme ? AvailableApplicationTheme.Dark : AvailableApplicationTheme.Light;
    }
}


Review comment (suggestion):
Hi @badcel ,
Do you think this is a reliable way to detect the dark theme? On Linux Mint, without Adw, `GtkApplicationPreferDarkTheme` is always false even if the system theme is dark.

Code after:
        {
            return true;
        }

        if (_mainWindow is not null)
        {
            return _mainWindow.GetWidth() <= NavBarThresholds.NavBarWidthSidebarCollapseThreshold;
        }

        return false;
    }

    private void UpdateSystemSettingsAndApplyTheme()
    {
        IsHighContrast
            = _gtkSettings.GtkThemeName is not null
              && _gtkSettings.GtkThemeName.Contains("high", StringComparison.OrdinalIgnoreCase)
              && _gtkSettings.GtkThemeName.Contains("contrast", StringComparison.OrdinalIgnoreCase);
        CurrentSystemTheme = GetCurrentSystemTheme();

        ApplyDesiredColorTheme();
    }

    private AvailableApplicationTheme GetCurrentSystemTheme()
    {
        return _gtkSettings.GtkApplicationPreferDarkTheme || (_gtkSettings.GtkThemeName?.Contains("Dark", StringComparison.OrdinalIgnoreCase) ?? false)
            ? AvailableApplicationTheme.Dark
            : AvailableApplicationTheme.Light;
    }
}
Repository: versotile-org/verso
File: src/config.rs (Rust)

Code before:
use std::{fs, path::PathBuf};

use embedder_traits::resources::{self, Resource, ResourceReaderMethods};
use headers::{ContentType, HeaderMapExt};
use net::protocols::{ProtocolHandler, ProtocolRegistry};
use net_traits::{
    request::Request,
    response::{Response, ResponseBody},
    ResourceFetchTiming,
};
use servo_config::opts::{default_opts, set_options, Opts};

/// Command line arguments.
#[derive(Clone, Debug, Default)]
pub struct CliArgs {
    /// Url to load initially.
    pub initial_url: Option<url::Url>,
}

/// Configuration of Verso instance.
#[derive(Clone, Debug)]
pub struct Config {
    /// Global flag options of Servo.
    pub opts: Opts,
    /// Path to resources directory.
    pub resource_dir: PathBuf,
    /// Command line arguments.
    pub cli_args: CliArgs,
}

fn parse_cli_args() -> Result<CliArgs, getopts::Fail> {
    let args: Vec<String> = std::env::args().collect();

    let mut opts = getopts::Options::new();
    opts.optopt("", "initial-url", "load this url on start", "url");

    let matches = opts.parse(&args[1..])?;
    let initial_url = matches
        .opt_str("initial-url")
        .and_then(|initial_url| match url::Url::parse(&initial_url) {
            Ok(url_parsed) => Some(url_parsed),
            Err(e) => {
                if e == url::ParseError::RelativeUrlWithoutBase {

Review comment (suggestion):
This is great! But could you allow me to rename it a bit?

```suggestion
pub struct Args {
    /// URL to load initially.
    pub url: Option<url::Url>,
}
```

Code after:
use std::{fs, path::PathBuf};

use embedder_traits::resources::{self, Resource, ResourceReaderMethods};
use headers::{ContentType, HeaderMapExt};
use net::protocols::{ProtocolHandler, ProtocolRegistry};
use net_traits::{
    request::Request,
    response::{Response, ResponseBody},
    ResourceFetchTiming,
};
use servo_config::opts::{default_opts, set_options, Opts};

/// Command line arguments.
#[derive(Clone, Debug, Default)]
pub struct CliArgs {
    /// URL to load initially.
    pub url: Option<url::Url>,
}

/// Configuration of Verso instance.
#[derive(Clone, Debug)]
pub struct Config {
    /// Global flag options of Servo.
    pub opts: Opts,
    /// Path to resources directory.
    pub resource_dir: PathBuf,
    /// Command line arguments.
    pub args: CliArgs,
}

fn parse_cli_args() -> Result<CliArgs, getopts::Fail> {
    let args: Vec<String> = std::env::args().collect();

    let mut opts = getopts::Options::new();
    opts.optopt("", "url", "URL to load on start", "URL");

    let matches = opts.parse(&args[1..])?;
    let initial_url = matches
        .opt_str("initial-url")
        .and_then(|initial_url| match url::Url::parse(&initial_url) {
            Ok(url_parsed) => Some(url_parsed),
            Err(e) => {
                if e == url::ParseError::RelativeUrlWithoutBase {
Repository: versotile-org/verso
File: src/config.rs (Rust)

Code before:
use std::{fs, path::PathBuf};

use embedder_traits::resources::{self, Resource, ResourceReaderMethods};
use headers::{ContentType, HeaderMapExt};
use net::protocols::{ProtocolHandler, ProtocolRegistry};
use net_traits::{
    request::Request,
    response::{Response, ResponseBody},
    ResourceFetchTiming,
};
use servo_config::opts::{default_opts, set_options, Opts};

/// Command line arguments.
#[derive(Clone, Debug, Default)]
pub struct CliArgs {
    /// URL to load initially.
    pub url: Option<url::Url>,
    /// URL to load initially.
    pub ipc_channel_name: Option<String>,
}

/// Configuration of Verso instance.
#[derive(Clone, Debug)]
pub struct Config {
    /// Global flag options of Servo.
    pub opts: Opts,
    /// Path to resources directory.
    pub resource_dir: PathBuf,
    /// Command line arguments.
    pub args: CliArgs,
}

fn parse_cli_args() -> Result<CliArgs, getopts::Fail> {
    let args: Vec<String> = std::env::args().collect();

    let mut opts = getopts::Options::new();
    opts.optopt("", "url", "URL to load on start", "URL");
    opts.optopt(
        "",
        "ipc-channel",
        "IPC channel name to communicate and control verso",
        "",
    );


Review comment (suggestion):
```suggestion
    pub ipc_channel: Option<String>,
```

Code after:
use std::{fs, path::PathBuf};

use embedder_traits::resources::{self, Resource, ResourceReaderMethods};
use headers::{ContentType, HeaderMapExt};
use net::protocols::{ProtocolHandler, ProtocolRegistry};
use net_traits::{
    request::Request,
    response::{Response, ResponseBody},
    ResourceFetchTiming,
};
use servo_config::opts::{default_opts, set_options, Opts};

/// Command line arguments.
#[derive(Clone, Debug, Default)]
pub struct CliArgs {
    /// URL to load initially.
    pub url: Option<url::Url>,
    /// URL to load initially.
    pub ipc_channel: Option<String>,
}

/// Configuration of Verso instance.
#[derive(Clone, Debug)]
pub struct Config {
    /// Global flag options of Servo.
    pub opts: Opts,
    /// Path to resources directory.
    pub resource_dir: PathBuf,
    /// Command line arguments.
    pub args: CliArgs,
}

fn parse_cli_args() -> Result<CliArgs, getopts::Fail> {
    let args: Vec<String> = std::env::args().collect();

    let mut opts = getopts::Options::new();
    opts.optopt("", "url", "URL to load on start", "URL");
    opts.optopt(
        "",
        "ipc-channel",
        "IPC channel name to communicate and control verso",
        "",
    );
Repository: versotile-org/verso
File: src/config.rs (Rust)

Code before:
#[derive(Clone, Debug)]
pub struct Config {
    /// Global flag options of Servo.
    pub opts: Opts,
    /// Path to resources directory.
    pub resource_dir: PathBuf,
    /// Command line arguments.
    pub args: CliArgs,
}

fn parse_cli_args() -> Result<CliArgs, getopts::Fail> {
    let args: Vec<String> = std::env::args().collect();

    let mut opts = getopts::Options::new();
    opts.optopt("", "url", "URL to load on start", "URL");
    opts.optopt(
        "",
        "ipc-channel",
        "IPC channel name to communicate and control verso",
        "",
    );
    opts.optflag("", "no-panel", "Launch Verso without control panel");
    opts.optopt(
        "",
        "devtools-port",
        "Launch Verso with devtools server enabled and listening on port",
        "port",
    );

    opts.optopt(
        "w",
        "width",
        "Initial window's width in physical unit, the height command line arg must also be set",
        "",
    );
    opts.optopt(
        "h",
        "height",
        "Initial window's height in physical unit, the width command line arg must also be set",
        "",
    );
    opts.optopt(
        "x",
        "",
        "Initial window's top left x position in physical unit, the y command line arg must also be set. Wayland isn't supported.",
        "",
    );
    opts.optopt(
        "y",
        "",
        "Initial window's top left y position in physical unit, the x command line arg must also be set. Wayland isn't supported.",

Review comment (suggestion):
```suggestion
        "Launch Verso with devtools server enabled and listen to port",
```

Code after:
#[derive(Clone, Debug)]
pub struct Config {
    /// Global flag options of Servo.
    pub opts: Opts,
    /// Path to resources directory.
    pub resource_dir: PathBuf,
    /// Command line arguments.
    pub args: CliArgs,
}

fn parse_cli_args() -> Result<CliArgs, getopts::Fail> {
    let args: Vec<String> = std::env::args().collect();

    let mut opts = getopts::Options::new();
    opts.optopt("", "url", "URL to load on start", "URL");
    opts.optopt(
        "",
        "ipc-channel",
        "IPC channel name to communicate and control verso",
        "",
    );
    opts.optflag("", "no-panel", "Launch Verso without control panel");
    opts.optopt(
        "",
        "devtools-port",
        "Launch Verso with devtools server enabled and listen to port",
        "port",
    );

    opts.optopt(
        "w",
        "width",
        "Initial window's width in physical unit, the height command line arg must also be set",
        "",
    );
    opts.optopt(
        "h",
        "height",
        "Initial window's height in physical unit, the width command line arg must also be set",
        "",
    );
    opts.optopt(
        "x",
        "",
        "Initial window's top left x position in physical unit, the y command line arg must also be set. Wayland isn't supported.",
        "",
    );
    opts.optopt(
        "y",
        "",
        "Initial window's top left y position in physical unit, the x command line arg must also be set. Wayland isn't supported.",
Repository: versotile-org/verso
File: src/config.rs (Rust)

Code before:
        _ => {}
    };

    Ok(CliArgs {
        url,
        resource_dir,
        ipc_channel,
        no_panel,
        window_attributes,
        devtools_port,
    })
}

impl Config {
    /// Create a new configuration for creating Verso instance. It must provide the path of
    /// resources directory.
    pub fn new() -> Self {
        let mut opts = default_opts();
        let mut args = parse_cli_args().unwrap_or_default();

        if let Some(devtools_port) = args.devtools_port {
            opts.devtools_server_enabled = true;
            opts.devtools_port = devtools_port;
        }

        let resource_dir = args.resource_dir.take().unwrap_or(resources_dir_path());

        Self {
            opts,
            args,
            resource_dir,
        }
    }

    /// Register URL scheme protocols
    pub fn create_protocols(&self) -> ProtocolRegistry {
        let handler = ResourceReader(self.resource_dir.clone());
        let mut protocols = ProtocolRegistry::with_internal_protocols();
        protocols.register("verso", handler);
        protocols
    }

    /// Init options and preferences.
    pub fn init(self) {
        // Set the resource files and preferences of Servo.
        resources::set(Box::new(ResourceReader(self.resource_dir.clone())));

        // Set the global options of Servo.
        set_options(self.opts);
    }
}

Review comment (suggestion):
We currently don't use this cli arg elsewhere but we might in the future, since this clone is trivial, I think it's better to just keep the `args` real to the real command line args

```suggestion
        let resource_dir = args.resource_dir.clone().unwrap_or_else(|| resources_dir_path());
```

Code after:
        _ => {}
    };

    Ok(CliArgs {
        url,
        resource_dir,
        ipc_channel,
        no_panel,
        window_attributes,
        devtools_port,
    })
}

impl Config {
    /// Create a new configuration for creating Verso instance. It must provide the path of
    /// resources directory.
    pub fn new() -> Self {
        let mut opts = default_opts();
        let args = parse_cli_args().unwrap_or_default();

        if let Some(devtools_port) = args.devtools_port {
            opts.devtools_server_enabled = true;
            opts.devtools_port = devtools_port;
        }

        let resource_dir = args.resource_dir.clone().unwrap_or(resources_dir_path());

        Self {
            opts,
            args,
            resource_dir,
        }
    }

    /// Register URL scheme protocols
    pub fn create_protocols(&self) -> ProtocolRegistry {
        let handler = ResourceReader(self.resource_dir.clone());
        let mut protocols = ProtocolRegistry::with_internal_protocols();
        protocols.register("verso", handler);
        protocols
    }

    /// Init options and preferences.
    pub fn init(self) {
        // Set the resource files and preferences of Servo.
        resources::set(Box::new(ResourceReader(self.resource_dir)));

        // Set the global options of Servo.
        set_options(self.opts);
    }
}
Repository: versotile-org/verso
File: verso/src/lib.rs (Rust)

Code before:
use std::{path::Path, process::Command};
use versoview_messages::ControllerMessage;

use ipc_channel::ipc::{IpcOneShotServer, IpcSender};

pub struct VersoviewController(IpcSender<ControllerMessage>);

impl VersoviewController {
    /// Create a new verso instance and get the controller to it
    pub fn new(verso_path: impl AsRef<Path>, initial_url: url::Url) -> Self {
        let path = verso_path.as_ref();
        let (server, server_name) =
            IpcOneShotServer::<IpcSender<ControllerMessage>>::new().unwrap();
        Command::new(path)
            .arg(format!("--ipc-channel={server_name}"))
            .arg(format!("--url={initial_url}"))
            .spawn()
            .unwrap();
        let (_, sender) = server.accept().unwrap();
        Self(sender)
    }

    /// Navigate to url
    pub fn navigate(&self, url: url::Url) -> Result<(), Box<ipc_channel::ErrorKind>> {
        self.0.send(ControllerMessage::NavigateTo(url))
    }
}


Review comment (suggestion):
I think we can leave this as `new`. If developers don't want to modify the settings, then they can just pass `VersoViewSettings::default()`

Code after:
use std::{path::Path, process::Command};
use versoview_messages::ControllerMessage;

use ipc_channel::ipc::{IpcOneShotServer, IpcSender};

#[derive(Debug, Default)]
pub struct VersoviewSettings {
    pub with_panel: bool,
}

pub struct VersoviewController(IpcSender<ControllerMessage>);

impl VersoviewController {
    /// Create a new verso instance with settings and get the controller to it
    fn create(
        verso_path: impl AsRef<Path>,
        initial_url: url::Url,
        settings: VersoviewSettings,
    ) -> Self {
        let path = verso_path.as_ref();
        let (server, server_name) =
            IpcOneShotServer::<IpcSender<ControllerMessage>>::new().unwrap();
        let mut command = Command::new(path);
        command
            .arg(format!("--ipc-channel={server_name}"))
            .arg(format!("--url={initial_url}"));
        if !settings.with_panel {
            command.arg("--no-panel");
        }
        command.spawn().unwrap();
        let (_, sender) = server.accept().unwrap();
        Self(sender)
    }

    /// Create a new verso instance with default settings and get the controller to it
    pub fn new(verso_path: impl AsRef<Path>, initial_url: url::Url) -> Self {
        Self::create(verso_path, initial_url, VersoviewSettings::default())
    }

    /// Create a new verso instance with custom settings and get the controller to it