Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Making every type optional via Option #4406

Closed
wants to merge 3 commits into from

Conversation

2022H1030042G
Copy link

@2022H1030042G 2022H1030042G commented Apr 24, 2023

Refer: #4399

Summary

Test Plan

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Apr 24, 2023

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 0b70902
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/64493cc63c86c300082c8fa3

@github-actions github-actions bot added the A-Project Area: project configuration and loading label Apr 24, 2023
@ematipico
Copy link
Contributor

ematipico commented Apr 25, 2023

@2022H1030042G have you tried running cargo check? That command will give you many suggestions on how to fix the compiler errors

@github-actions github-actions bot added the A-Formatter Area: formatter label Apr 26, 2023
@@ -83,7 +83,7 @@ pub enum IndentStyle {
#[default]
Tab,
/// Space, with its quantity
Space(u8),
Space(Option<u8>),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't need to change. You can revert the change

@@ -74,7 +74,7 @@ impl PrinterOptions {
pub(super) const fn indent_width(&self) -> u8 {
match self.indent_style {
IndentStyle::Tab => self.tab_width,
IndentStyle::Space(count) => count,
IndentStyle::Space(Option<count>) => count,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can revert the change here

@@ -153,7 +153,7 @@ struct IrFormatOptions;

impl FormatOptions for IrFormatOptions {
fn indent_style(&self) -> IndentStyle {
IndentStyle::Space(2)
IndentStyle::Space(Some(2))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can revert the change

@@ -165,7 +165,7 @@ impl FormatOptions for IrFormatOptions {
tab_width: 2,
print_width: self.line_width().into(),
line_ending: LineEnding::LineFeed,
indent_style: IndentStyle::Space(2),
indent_style: IndentStyle::Space(Some(2)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can revert the change

Comment on lines +75 to +76
Some(PlainIndentStyle::Tab) => IndentStyle::Tab,
Some(PlainIndentStyle::Space) => IndentStyle::Space(conf.indent_size),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some(PlainIndentStyle::Tab) => IndentStyle::Tab,
Some(PlainIndentStyle::Space) => IndentStyle::Space(conf.indent_size),
Some(PlainIndentStyle::Tab) => IndentStyle::Tab,
Some(PlainIndentStyle::Space) => IndentStyle::Space(conf.indent_size),
None => IndentStyle::Tab

where
D: serde::de::Deserializer<'de>,
{
let value: u16 = Deserialize::deserialize(deserializer)?;
LineWidth::try_from(value).map_err(serde::de::Error::custom)
Option::<LineWidth>::try_from(value).map_err(serde::de::Error::custom)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Option::<LineWidth>::try_from(value).map_err(serde::de::Error::custom)
let line_width = LineWidth::try_from(value).map_err(serde::de::Error::custom)?;
Ok(Some(line_width))

@2022H1030042G
Copy link
Author

2022H1030042G commented Apr 26, 2023

After performing the above changes, there are still errors. PFA
errors.txt

@ematipico
Copy link
Contributor

After performing the above changes, there are still errors. PFA errors.txt

You can push the changes :) no need to attach any files!

@github-actions github-actions bot removed the A-Formatter Area: formatter label Apr 26, 2023
@2022H1030042G
Copy link
Author

2022H1030042G commented Apr 26, 2023

After performing the above changes, there are still errors. PFA errors.txt

You can push the changes :) no need to attach any files!

Done. Will you be looking into it?

@ematipico
Copy link
Contributor

Hi @2022H1030042G, I thought you had some basic knowledge of Rust. I needed this feature for #4405 and eventually did it myself there.

Thank you for your contribution. I am going to create some new good first issues that don't require priority from our end!

@ematipico ematipico closed this Apr 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Project Area: project configuration and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants