Skip to content

Conversation

Koooooo-7
Copy link
Member

@Koooooo-7 Koooooo-7 commented Feb 6, 2020

Summary
Trying fix #1009 emojis in titles not working correctly ,

I attempt to think all the types likes :words: as an emoji code that I wont replace it to empty(' ').
At the same time, it causes another proplem that it cant solve foo::ddd::ccc as u want ever, the same as the time 15:02:55. BUT i think it is acceptable, because :
A: In my opnion, we doesnt render the foo::ddd::ccc 's title id correctly now.
B: I think it is a good idea to put them within a code backticks that we can distinguish the title ,the emojis and the configurations easily at now.

Here's an idea - for the weird code syntax cases that #708 was trying to fix, require users to put them within a code backticks to avoid any conflicting treatment from config or emojis, for example:
## `MyPerlModule::Submodule::function` :emoji: :id=something

TEST CASE

  • PASS
#### :arrow_forward: :arrow_forward: #### :arrow_forward::arrow_forward: ####  :arrow_forward: :arrow_forward: ####  :arrow_forward::arrow_forward: ####  :arrow_forward: :arrow_forward: # `MyPerlModule::Submodule::function` :dog: :dog: :id=something ### `15:22:10` # `foo::bar::baz` # `foo:::bar:::baz` 
  • UNPASS
### 15:22:10 ### foo::bar::baz ### foo:::bar:::baz 

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:


  • DO NOT include files inside lib directory.
@anikethsaha
Copy link
Member

I will review it soon

@anikethsaha anikethsaha self-assigned this Feb 7, 2020
@anikethsaha anikethsaha requested review from jthegedus, trusktr and a team February 7, 2020 18:13
Copy link
Member

@anikethsaha anikethsaha left a comment

Choose a reason for hiding this comment

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

Well done 🎉

Just one note, can you document this. It will be good for users.

@anikethsaha
Copy link
Member

@Koooooo-7 Thanks a lot for your time and the fix. The fix looks good.

Just need to add notes in the docs for future ref !!!

Copy link
Contributor

@jthegedus jthegedus left a comment

Choose a reason for hiding this comment

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

LGTM

@Koooooo-7
Copy link
Member Author

@anikethsaha
I guess I didn't add the note in a wrong place . 🐶

Copy link
Member

@anikethsaha anikethsaha left a comment

Choose a reason for hiding this comment

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

Nice 🥇
Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants