Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layer API improvements and fixups #121

Merged
merged 1 commit into from
May 12, 2021
Merged

Layer API improvements and fixups #121

merged 1 commit into from
May 12, 2021

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented May 11, 2021

  • Added LayerSet::new
  • Added Layer::rename_glyph
  • Fixed bug where get_glyph_mut used Arc::get_mut instead of Arc::make_mut
  • Renamed get_glyph to glyph
  • Renamed get_glyph_mut to glyph_mut
  • Renamed iter_contents & iter_contents_mut -> iter & iter_mut
  • Removed deprecated method

A few other things along these lines


this is also split off from #114

@github-actions
Copy link

🗜 Bloat check ⚖️

Comparing 0e56be1 against e02654c

target old size new size difference
target/release/examples/open_ufo 1.69 MB 1.69 MB -2.07 KB (-0.12%)
target/debug/examples/open_ufo 7.07 MB 7.07 MB -392 Bytes (-0.01%)

@github-actions
Copy link

🗜 Bloat check ⚖️

Comparing 3185037 against e02654c

target old size new size difference
target/release/examples/open_ufo 1.69 MB 1.69 MB -2.07 KB (-0.12%)
target/debug/examples/open_ufo 7.07 MB 7.07 MB -392 Bytes (-0.01%)

src/layer.rs Show resolved Hide resolved
src/layer.rs Outdated
@@ -350,7 +359,7 @@ impl Layer {
}

/// Returns a reference the glyph with the given name, if it exists.
pub fn get_glyph<K>(&self, glyph: &K) -> Option<&Arc<Glyph>>
pub fn glyph<K>(&self, glyph: &K) -> Option<&Arc<Glyph>>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't get be more appropriate? Layers are basically HashMaps with fixed types...

Copy link
Member Author

Choose a reason for hiding this comment

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

I think there are enough other things hanging of a layer that just 'get' would be unclear; it could be the layer's name, or it's path, or something from the lib...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then I actually advocate keeping "get_glyph" because it is clear. Otherwise, I'll bite my tongue now.

src/layer.rs Outdated Show resolved Hide resolved
@github-actions
Copy link

🗜 Bloat check ⚖️

Comparing c58713e against 92b2837

target old size new size difference
target/release/examples/open_ufo 1.71 MB 1.71 MB -1.17 KB (-0.07%)
target/debug/examples/open_ufo 7.12 MB 7.12 MB -824 Bytes (-0.01%)

@github-actions
Copy link

🗜 Bloat check ⚖️

Comparing 383a2a1 against 2156689

target old size new size difference
target/release/examples/open_ufo 1.71 MB 1.71 MB -1.16 KB (-0.07%)
target/debug/examples/open_ufo 7.14 MB 7.14 MB 704 Bytes (0.01%)

- Added LayerSet::new
- Added Layer::rename_glyph
- Fixed bug where get_glyph_mut used Arc::get_mut instead of Arc::make_mut
- Renamed iter_contents & iter_contents_mut -> iter & iter_mut
- Removed deprecated method

A few other things along these lines
@github-actions
Copy link

🗜 Bloat check ⚖️

Comparing c0174c7 against 2156689

target old size new size difference
target/release/examples/open_ufo 1.71 MB 1.71 MB -1.16 KB (-0.07%)
target/debug/examples/open_ufo 7.14 MB 7.14 MB 704 Bytes (0.01%)

@cmyr cmyr merged commit 96c5fc9 into master May 12, 2021
@cmyr cmyr deleted the layer-api-improvements branch May 12, 2021 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants