Skip to content
This repository has been archived by the owner on Jul 23, 2024. It is now read-only.

Refactor shell theming module #806

Merged
merged 5 commits into from
Aug 16, 2023
Merged

Refactor shell theming module #806

merged 5 commits into from
Aug 16, 2023

Conversation

lavafroth
Copy link
Contributor

@lavafroth lavafroth commented Aug 15, 2023

Description

This PR refactors a few operations to make the code handle edge cases efficiently with marginal performance improvements.

Type of change

  • Bugfix (Change which fixes an issue)
  • New feature (Change which adds new functionality)
  • Enhancement (Change which slightly improves existing code)
  • Breaking change (This change will introduce incompatibility with existing functionality)

Changelog

  • Stores the comma separated string representation of valid versions for later use in raising exceptions.
  • Uses tuple for async_data in ShellTheme since its members are immutable.
  • Uses tuple decomposition to assign theme variant and preset from async_data later.
  • Replaces Gio.File.make_directory_with_parents with Python's builtin os.makedirs with exist_ok as True to implicitly create directories when nonexistent.
  • Instead of relying on KeyErrors when accessing dictionary values, the get method is used to supply a default value.
  • When a key might exist in one dictionary, the get method (which may return None) is coupled with the nullish or operator to use the value that is not None.
  • Uses predefined main_source attribute as argument to the _compile_sass function.
  • Uses the walrus operator to scope a template match to the condition when it is not None.
  • Removes unnecessary implicit .close()s on file handles when using context managers.

Testing

  • I have tested my changes and verified that they work as expected

How to test the changes

Just run ./local.sh

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome on Gradience. 🥳 We really appreciate your contribution. The core team will review your pull request as soon as possible. You can also join the Matrix room: https://matrix.to/#/#Gradience-space:envs.net or the Discord server: https://discord.com/invite/4njFDtfGEZ.

@daudix daudix requested a review from tfuxu August 15, 2023 13:23
Copy link
Member

@tfuxu tfuxu left a comment

Choose a reason for hiding this comment

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

Thanks for this refactoring job @lavafroth! Overall looks good, but I'll do a proper review tomorrow

Copy link
Member

@tfuxu tfuxu left a comment

Choose a reason for hiding this comment

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

Alright, I've tested everything related to this refactor and it works flawlessly.

Thanks again for this refactor, I didn't thought anyone would be able to decipher this mess I wrote here 😅️

@tfuxu
Copy link
Member

tfuxu commented Aug 16, 2023

@0xMRTT Any objections, or can we merge this?

@0xMRTT
Copy link
Member

0xMRTT commented Aug 16, 2023

LGTM We can merge ! Thanks for clearing the mess lol

@0xMRTT 0xMRTT merged commit db124a2 into GradienceTeam:main Aug 16, 2023
@lavafroth lavafroth deleted the refactor-shell branch August 17, 2023 06:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants