ronantakizawa/codereview-bench huggingface dataset:
Repository: nushell/nushell
File: crates/nu-color-config/src/nu_style.rs (Rust)

Code before:
pub struct NuStyle {
    pub fg: Option<String>,
    pub bg: Option<String>,
    pub attr: Option<String>,
}

impl From<Style> for NuStyle {
    fn from(s: Style) -> Self {
        Self {
            bg: s.background.and_then(color_to_string),
            fg: s.foreground.and_then(color_to_string),
            attr: style_get_attr(s),
        }
    }
}

fn style_get_attr(s: Style) -> Option<String> {
    let mut attrs = String::new();

    if s.is_blink {
        attrs.push('l');
    };
    if s.is_bold {
        attrs.push('b');
    };
    if s.is_dimmed {
        attrs.push('d');
    };
    if s.is_hidden {
        attrs.push('h');
    };
    if s.is_italic {
        attrs.push('i');
    };
    if s.is_reverse {
        attrs.push('r');
    };
    if s.is_strikethrough {
        attrs.push('s');
    };
    if s.is_underline {
        attrs.push('u');
    };

    if attrs.is_empty() { None } else { Some(attrs) }
}

fn color_to_string(color: Color) -> Option<String> {
    match color {
        Color::Black => Some(String::from("black")),
        Color::DarkGray => Some(String::from("dark_gray")),

Review comment (suggestion):
Any way we can support the shortest non-conflicting prefix here? (`bo` = bold, `bl` = blink, and such).
This behaviour is already present in `(ansi bl)` which _only_ makes it blink.

Code after:
pub struct NuStyle {
    pub fg: Option<String>,
    pub bg: Option<String>,
    pub attr: Option<String>,
}

// Valid attribute codes and their corresponding names
const VALID_CODES: &str = "lbhdirstun"; // blink, bold, dimmed, hidden, italic, reverse, strike, underline, normal
const VALID_CODE_NAMES: &[(&str, char)] = &[
    ("blink", 'l'),
    ("bold", 'b'),
    ("dimmed", 'd'),
    ("hidden", 'h'),
    ("italic", 'i'),
    ("reverse", 'r'),
    ("strike", 's'),
    ("strikethrough", 's'),
    ("underline", 'u'),
    ("normal", 'n'),
];

const VALID_CODES_HELP: &str = "Valid codes are: b (bold), i (italic), u (underline), s (strike), d (dimmed), r (reverse), h (hidden), l (blink), n (normal)";
const VALID_NAMES_HELP: &str =
    "Valid names are: bold, italic, underline, strike, dimmed, reverse, hidden, blink, normal";

fn name_to_code(name: &str) -> Option<char> {
    VALID_CODE_NAMES
        .iter()
        .find(|(n, _)| *n == name)
        .map(|(_, c)| *c)
}

impl From<Style> for NuStyle {
    fn from(s: Style) -> Self {
        Self {
            bg: s.background.and_then(color_to_string),
            fg: s.foreground.and_then(color_to_string),
            attr: style_get_attr(s),
        }
    }
}

fn style_get_attr(s: Style) -> Option<String> {
    let mut attrs = String::new();

    if s.is_blink {
        attrs.push('l');
    }
    if s.is_bold {
        attrs.push('b');
    }
Repository: nushell/nushell
File: crates/nu_plugin_formats/src/from/ini.rs (Rust)

Code before:
use crate::FormatCmdsPlugin;

use nu_plugin::{EngineInterface, EvaluatedCall, SimplePluginCommand};
use nu_protocol::{
    Category, Example, LabeledError, Record, ShellError, Signature, Type, Value, record,
};

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values", None)
            .switch(
                "no-escape",
                "Disable escape sequence handling for values",
                None,
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines",
                None,
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys",
                None,
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,

Review comment (security):
I think it might be better if these 2 no-blah flags are not negatives. So `--quote` and `--escape` vs `--no-quote` and `--no-escape`. Especially since the last two are positive flags vs negative flags like these. Being consistent would help I think.

Code after:
use crate::FormatCmdsPlugin;

use nu_plugin::{EngineInterface, EvaluatedCall, SimplePluginCommand};
use nu_protocol::{
    Category, Example, LabeledError, Record, ShellError, Signature, Type, Value, record,
};

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values.", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values.",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines.",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys.",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
Repository: nushell/nushell
File: crates/nu_plugin_formats/src/from/ini.rs (Rust)

Code before:
use crate::FormatCmdsPlugin;

use nu_plugin::{EngineInterface, EvaluatedCall, SimplePluginCommand};
use nu_protocol::{
    Category, Example, LabeledError, Record, ShellError, Signature, Type, Value, record,
};

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,

Review comment (suggestion):
This is a nit but these need to start with a capital letter (which they do) and end with a period (which they don't). We've just had 10 PRs to try and standardize this with the last PR unmerged right now.

```suggestion
            .switch("no-quote", "Disable quote handling for values.", Some('q'))
```

Code after:
use crate::FormatCmdsPlugin;

use nu_plugin::{EngineInterface, EvaluatedCall, SimplePluginCommand};
use nu_protocol::{
    Category, Example, LabeledError, Record, ShellError, Signature, Type, Value, record,
};

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values.", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values.",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines.",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys.",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
Repository: nushell/nushell
File: crates/nu_plugin_formats/src/from/ini.rs (Rust)

Code before:

use nu_plugin::{EngineInterface, EvaluatedCall, SimplePluginCommand};
use nu_protocol::{
    Category, Example, LabeledError, Record, ShellError, Signature, Type, Value, record,
};

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
        _engine: &EngineInterface,
        call: &EvaluatedCall,
        input: &Value,

Review comment (security):
```suggestion
                "Disable escape sequence handling for values.",
```

Code after:

use nu_plugin::{EngineInterface, EvaluatedCall, SimplePluginCommand};
use nu_protocol::{
    Category, Example, LabeledError, Record, ShellError, Signature, Type, Value, record,
};

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values.", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values.",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines.",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys.",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
        _engine: &EngineInterface,
        call: &EvaluatedCall,
        input: &Value,
Repository: nushell/nushell
File: crates/nu_plugin_formats/src/from/ini.rs (Rust)

Code before:

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
        _engine: &EngineInterface,
        call: &EvaluatedCall,
        input: &Value,
    ) -> Result<Value, LabeledError> {
        let span = input.span();
        let input_string = input.coerce_str()?;
        let head = call.head;
        let mut parse_option = ini::ParseOption::default();

Review comment (suggestion):
```suggestion
                "Allow values to continue on indented lines.",
```

Code after:

pub struct FromIni;

impl SimplePluginCommand for FromIni {
    type Plugin = FormatCmdsPlugin;

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values.", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values.",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines.",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys.",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
        _engine: &EngineInterface,
        call: &EvaluatedCall,
        input: &Value,
    ) -> Result<Value, LabeledError> {
        let span = input.span();
        let input_string = input.coerce_str()?;
        let head = call.head;
        let mut parse_option = ini::ParseOption::default();
Repository: nushell/nushell
File: crates/nu_plugin_formats/src/from/ini.rs (Rust)

Code before:

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
        _engine: &EngineInterface,
        call: &EvaluatedCall,
        input: &Value,
    ) -> Result<Value, LabeledError> {
        let span = input.span();
        let input_string = input.coerce_str()?;
        let head = call.head;
        let mut parse_option = ini::ParseOption::default();

        if call.has_flag("no-quote")? {
            parse_option.enabled_quote = false;
        }


Review comment (suggestion):
```suggestion
                "Preserve leading whitespace in keys.",
```

Code after:

    fn name(&self) -> &str {
        "from ini"
    }

    fn description(&self) -> &str {
        "Parse text as .ini and create table."
    }

    fn signature(&self) -> Signature {
        Signature::build(self.name())
            .input_output_types(vec![(Type::String, Type::record())])
            .switch("no-quote", "Disable quote handling for values.", Some('q'))
            .switch(
                "no-escape",
                "Disable escape sequence handling for values.",
                Some('e'),
            )
            .switch(
                "indented-multiline-value",
                "Allow values to continue on indented lines.",
                Some('m'),
            )
            .switch(
                "preserve-key-leading-whitespace",
                "Preserve leading whitespace in keys.",
                Some('w'),
            )
            .category(Category::Formats)
    }

    fn examples(&self) -> Vec<Example<'_>> {
        examples()
    }

    fn run(
        &self,
        _plugin: &FormatCmdsPlugin,
        _engine: &EngineInterface,
        call: &EvaluatedCall,
        input: &Value,
    ) -> Result<Value, LabeledError> {
        let span = input.span();
        let input_string = input.coerce_str()?;
        let head = call.head;
        let mut parse_option = ini::ParseOption::default();

        if call.has_flag("no-quote")? {
            parse_option.enabled_quote = false;
        }
Repository: nuxt/nuxt
File: packages/nuxt/src/app/plugins/payload.client.ts (TypeScript)

Code before:
import { onNuxtReady } from '#app/composables/ready'
import { useRouter } from '#app/composables/router'
import { getAppManifest } from '#app/composables/manifest'

export default defineNuxtPlugin({
  name: 'nuxt:payload',
  setup (nuxtApp) {
    // TODO: Support dev
    if (process.dev) { return }

    // Load payload after middleware & once final route is resolved
    useRouter().beforeResolve(async (to, from) => {
      if (to.path === from.path) { return }
      const payload = await loadPayload(to.path)
      if (!payload) { return }
      Object.assign(nuxtApp.static.data, payload.data)
    })

    onNuxtReady(() => {
      // Load payload into cache
      nuxtApp.hooks.hook('link:prefetch', async (url) => {
        if (!parseURL(url).protocol) {
          await loadPayload(url)
        }
      })
      setTimeout(getAppManifest, 1000)
    })
  }
})


Review comment (performance):
Would be nice if we also checked the connection speed as well. Performance "hopefully" is okay because hopefully, mobiles load the critical path in less than a second. But i am more worried about paid internet connections that waste bandwidth if they don't need the manifest.

Code after:
import { onNuxtReady } from '#app/composables/ready'
import { useRouter } from '#app/composables/router'
import { getAppManifest } from '#app/composables/manifest'
// @ts-expect-error virtual file
import { appManifest as isAppManifestEnabled } from '#build/nuxt.config.mjs'

export default defineNuxtPlugin({
  name: 'nuxt:payload',
  setup (nuxtApp) {
    // TODO: Support dev
    if (process.dev) { return }

    // Load payload after middleware & once final route is resolved
    useRouter().beforeResolve(async (to, from) => {
      if (to.path === from.path) { return }
      const payload = await loadPayload(to.path)
      if (!payload) { return }
      Object.assign(nuxtApp.static.data, payload.data)
    })

    onNuxtReady(() => {
      // Load payload into cache
      nuxtApp.hooks.hook('link:prefetch', async (url) => {
        if (!parseURL(url).protocol) {
          await loadPayload(url)
        }
      })
      if (isAppManifestEnabled && navigator.connection?.effectiveType !== 'slow-2g') {
        setTimeout(getAppManifest, 1000)
      }
    })
  }
})
Repository: nuxt/nuxt
File: packages/schema/src/config/experimental.ts (TypeScript)

Code before:

    /**
     * Whether to call and use the result from `getCachedData` on manual refresh for `useAsyncData` and `useFetch`.
     */
    granularCachedData: {
      $resolve: async (val, get) => {
        return typeof val === 'boolean' ? val : ((await get('future')).compatibilityVersion === 4)
      },
    },

    /**
     * Whether to parse `error.data` when rendering a server error page.
     */
    parseErrorData: {
      $resolve: async (val, get) => {
        return typeof val === 'boolean' ? val : (await get('future')).compatibilityVersion === 4
      },
    },

    /**
     * Whether Nuxt should stop if a Nuxt module is incompatible.
     */
    enforceModuleCompatibility: false,

    /**
     * For `useAsyncData` and `useFetch`, whether `pending` should be true when data has not yet started to be fetched.
     */
    pendingWhenIdle: {
      $resolve: async (val, get) => {
        return typeof val === 'boolean' ? val : (await get('future')).compatibilityVersion !== 4
      },
    },
  },
})


Review comment (suggestion):
```suggestion
     * For `useAsyncData` and `useFetch`, whether `pending` should be `true` when data has not yet started to be fetched.
```

Code after:

    /**
     * Whether to call and use the result from `getCachedData` on manual refresh for `useAsyncData` and `useFetch`.
     */
    granularCachedData: {
      $resolve: async (val, get) => {
        return typeof val === 'boolean' ? val : ((await get('future')).compatibilityVersion === 4)
      },
    },

    /**
     * Whether to parse `error.data` when rendering a server error page.
     */
    parseErrorData: {
      $resolve: async (val, get) => {
        return typeof val === 'boolean' ? val : (await get('future')).compatibilityVersion === 4
      },
    },

    /**
     * Whether Nuxt should stop if a Nuxt module is incompatible.
     */
    enforceModuleCompatibility: false,

    /**
     * For `useAsyncData` and `useFetch`, whether `pending` should be `true` when data has not yet started to be fetched.
     */
    pendingWhenIdle: {
      $resolve: async (val, get) => {
        return typeof val === 'boolean' ? val : (await get('future')).compatibilityVersion !== 4
      },
    },
  },
})