Format from/to custom number base #430

youkai95 · 2022-02-27T03:51:52Z

Ok, now I think it has all the required functionalities. Here I left some features available like allowing formatting even for those base numbers using custom dictionaries. That idea could be improved as it would be nice to allow user to change the default character and group size... Yes, it is hardcoded at the moment.

I am not sure at all if I faced this problem in a correct way. If you have any comment regarding my code, please share it with me to improve it.

Pull request type

Please check the type of change your PR introduces:

Bugfix
Feature
UI change (please include screenshot!)
Code style update (formatting, renaming)
Refactoring (no functional changes, no api changes)
Build related changes
Documentation content changes
Internationalization and localization
Other (please describe):

What is the current behavior?

Issue Number: resolve #384

What is the new behavior?

Convert numbers from/to different bases
Provide and advanced view to allow complex functionalities.
Allows formatting for custom dictionaries (need work)

Other information

Quality check

Before creating this PR, have you:

Followed the code style guideline as described in CONTRIBUTING.md
Verified that the change work in Release build configuration
Checked all unit tests pass

youkai95 · 2022-03-06T01:10:43Z

.../impl/DevToys/Views/Tools/Converters/NumberBaseConverter/NumberBaseConverterToolPage.xaml.cs

-            if (!string.IsNullOrWhiteSpace(parameters.ClipBoardContent))
-            {
-                string clipBoardContent = NumberBaseFormatter.RemoveFormatting(parameters.ClipBoardContent).ToString();
+            // TODO: This feature would be nice to find a way to keep it
+            //if (!string.IsNullOrWhiteSpace(parameters.ClipBoardContent))
+            //{
+            //    string clipBoardContent = NumberBaseFormatter.RemoveFormatting(parameters.ClipBoardContent).ToString();

-                if (NumberBaseHelper.IsValidBinary(clipBoardContent!))
-                {
-                    ViewModel.InputBaseNumber = NumberBaseFormat.Binary;
-                }
-                else if (NumberBaseHelper.IsValidHexadecimal(clipBoardContent!))
-                {
-                    ViewModel.InputBaseNumber = NumberBaseFormat.Hexadecimal;
-                }
+            //    if (NumberBaseHelper.IsValidBinary(clipBoardContent!))
+            //    {
+            //        ViewModel.InputBaseNumber = NumberBaseFormat.Binary;
+            //    }
+            //    else if (NumberBaseHelper.IsValidHexadecimal(clipBoardContent!))
+            //    {
+            //        ViewModel.InputBaseNumber = NumberBaseFormat.Hexadecimal;
+            //    }

-                ViewModel.InputValue = parameters.ClipBoardContent;
-            }
+            //    ViewModel.InputValue = parameters.ClipBoardContent;
+            //}


It would be good to revert this changes and keep this feature. This could require big changes but if you give me any idea I will try to fix it.

What's the challenge you're encounter here? Is the problem to determine whether it's an advanced mode or not?

Yes, the tool opens through NumberBaseConverter main view but the functionalities are inside Basic/Advanced view models, who are not declared inside the main view. However, I just realized that I could get this done using messages, isn't it? Still remains the problem of selecting between basic or advanced view...

Ah I see. You should be able to do this by:

Set x:Name="AdvancedNumberBaseConverterControl" on the advanced control in the main view

And then, something like:

ViewModel.AdvancedMode = true; AdvancedNumberBaseConverterControl.ViewModel.Property = something;

I will keep this feature as it is at the moment. To include advanced conversions into this feature could be really confusing, so I will keep it working with basic conversions. I could later open an issue if we want to take this further.

* Added the possibility to Favorite/Unfavorite a tool. * Updated localization * addressed feedback

youkai95 · 2022-03-08T16:45:39Z

@veler @btiteux this PR is ready to review 😁 There are few details that would be nice to improve, but overall, it is ready to check.

veler · 2022-03-12T19:05:46Z

@veler @btiteux this PR is ready to review 😁 There are few details that would be nice to improve, but overall, it is ready to check.

Hello :)
Sorry for the very late answer. I'm still very busy lately. I will take a look now. :)

veler · 2022-03-12T19:18:55Z

Custom base doesn't seem to work, or I don't understand how it should work.

I'm also facing some error banner when enabling/disabling Format number.
Also sometimes I'd type something in the Basic mode input and would get an error about the fact that the Custom Base dictionary is wrong. You should really separate the view model of the Basic and Advanced mode so they don't conflict.

veler

Quite a big PR, good job :)
I encountered some bugs though, or didn't understand how to use it (in particular the custom base).

I'm also hitting some bugs that seems to come from the Advanced mode when I'm editing something in Basic mode (for example, the Format number option).

Thank you :)

...dev/impl/DevToys/Views/Tools/Converters/NumberBaseConverter/NumberBaseConverterToolPage.xaml

...impl/DevToys/Views/Tools/Converters/NumberBaseConverter/BasicNumberBaseConverterControl.xaml

...l/DevToys/Views/Tools/Converters/NumberBaseConverter/AdvancedNumberBaseConverterControl.xaml

src/dev/impl/DevToys/ViewModels/Tools/Converters/NumberBaseConverter/NumberBaseFormatFactory.cs

src/dev/impl/DevToys/Models/NumberBaseDictionary.cs

src/dev/impl/DevToys/Messages/ChangeNumberFormattingMessage.cs

src/dev/impl/DevToys/Messages/ChangeInfoBarStatusMessage.cs

youkai95 · 2022-03-13T04:17:38Z

Custom base doesn't seem to work, or I don't understand how it should work.

If you use a custom dictionary for input, then you should write an input value using that dictionary. Using you example, a correct input should be a value with characters:

 1234abcd
(01234567)

The line below correspond to the values that every character has when converting to decimal base.

I'm also facing some error banner when enabling/disabling Format number. Also sometimes I'd type something in the Basic mode input and would get an error about the fact that the Custom Base dictionary is wrong. You should really separate the view model of the Basic and Advanced mode so they don't conflict.

I am not sure about how to replicate this error. I would be grateful if you could provide me an example to simulate it. I have decided to keep the info box into the parent component to avoid having duplicated code as it is the same behavior for both views.

* Add StartWithLoremIpsum option * Updated language bundles * Added refresh buttom for lorem ipsum * Replaced click handler with Refresh ICommand * Fixing error Co-authored-by: Guus Beltman <[email protected]>

youkai95 · 2022-03-21T02:35:13Z

@veler the reviews you asked for should be solved with this push, sorry for the late. I also found some bugs and misbehavior in the code that should be solved now.

veler · 2022-03-21T16:44:40Z

Custom base doesn't seem to work, or I don't understand how it should work.

If you use a custom dictionary for input, then you should write an input value using that dictionary. Using you example, a correct input should be a value with characters:
 1234abcd
(01234567)
The line below correspond to the values that every character has when converting to decimal base.

I'm also facing some error banner when enabling/disabling Format number. Also sometimes I'd type something in the Basic mode input and would get an error about the fact that the Custom Base dictionary is wrong. You should really separate the view model of the Basic and Advanced mode so they don't conflict.

I am not sure about how to replicate this error. I would be grateful if you could provide me an example to simulate it. I have decided to keep the info box into the parent component to avoid having duplicated code as it is the same behavior for both views.

I see! Thanks for the explanation! It wasn't clear to me by looking at the UI that works this way. Should we consider clarifying it in the UI? What do yout think?

@veler the reviews you asked for should be solved with this push, sorry for the late. I also found some bugs and misbehavior in the code that should be solved now.

Thank you for this too. I gave it a try and it seems better :D
I added a few other comments to the code. Can you please address them when you have some time?
There's also a merge conflict to address.

After all this, I think it will be good to merge.

Thanks again!

youkai95 · 2022-03-21T17:14:10Z

Should we consider clarifying it in the UI? What do yout think?

I think that would be a good idea. If you were aware of this tool capabilities and you got confused, what could we expect from others? XD Also, I will try to discover what is happening with the label that doesnt get shown.

I added a few other comments to the code. Can you please address them when you have some time?
There's also a merge conflict to address.

Ok, I could address them in few time and do the merge. I also noticed that I miss some old changes...

veler

Hello,
Sorry for the delay.
It looks great now ! :D I will merge it. Thank you for the contribution, much appreciated! :)

src/dev/impl/DevToys/ViewModels/Tools/Converters/NumberBaseConverter/NumberBaseFormatBuilder.cs

src/dev/impl/DevToys/ViewModels/Tools/Converters/NumberBaseConverter/NumberBaseFormatter.cs

src/tests/DevToys.Tests/Providers/Tools/BaseNumberFormatterTests.cs

...evToys/Views/Tools/Converters/NumberBaseConverter/AdvancedNumberBaseConverterControl.xaml.cs

* Number conversion using custom bases functionalities and few tests * Functionalities to format numbers using any base and dictionary * Add new language text * Change models to fit new features * Add new communication messages between number converter view models * Add views and functionalities to allow custom dictionaries * Activate model to receive messages * Include language generated component * Rebase and cleanup code * Update InfoBar message to show/hide it correctly * Remove remaining code generating compile errors * Code refactoring * Fix test cases * Added the possibility to Favorite/Unfavorite a tool. (#437) * Added the possibility to Favorite/Unfavorite a tool. * Updated localization * addressed feedback * Fix README.md minor typo (#441) * Update japanese translation. (#444) * Bumped up version number * Feature/add refresh button lorem ipsum (#413) * Add StartWithLoremIpsum option * Updated language bundles * Added refresh buttom for lorem ipsum * Replaced click handler with Refresh ICommand * Fixing error Co-authored-by: Guus Beltman <[email protected]> * Added some assets * Updated chocolatey * updated wikiUrl in Chocolatey * Update Traditional Chinese translation (#442) * Updates to cs-CZ localization (#447) * feature: 💬 add new lang (Portuguese-Brazil) (#449) * Update japanese translation. (lorem ipsum) (#450) * Update japanese translation. (lorem ipsum) * Fix typo. * Update chinese translation (#451) * Added String Escape / Unescape tool (#446) * Set nullable * Set nullable and change to string dictionaries * Fix errors, inject view models and improve code * Add translations * Number conversion using custom bases functionalities and few tests * Functionalities to format numbers using any base and dictionary * Add new language text * Change models to fit new features * Add new communication messages between number converter view models * Add views and functionalities to allow custom dictionaries * Activate model to receive messages * Include language generated component * Rebase and cleanup code * Update InfoBar message to show/hide it correctly * Remove remaining code generating compile errors * Code refactoring * Fix test cases * Set nullable * Set nullable and change to string dictionaries * Fix errors, inject view models and improve code * Add translations * Automatic input when navigating to the tool Co-authored-by: Etienne BAUDOUX <[email protected]> Co-authored-by: kspc1000 <[email protected]> Co-authored-by: Sou Niyari <[email protected]> Co-authored-by: Guus Beltman <[email protected]> Co-authored-by: Guus Beltman <[email protected]> Co-authored-by: SiderealArt <[email protected]> Co-authored-by: Morning4coffe <[email protected]> Co-authored-by: Rafael Andrade de Oliveira <[email protected]> Co-authored-by: Boring3 <[email protected]>

youkai95 changed the title ~~Custom number base~~ Format from/to custom number base Feb 27, 2022

youkai95 mentioned this pull request Feb 27, 2022

Number Base Converter - Custom base #384

Closed

youkai95 added 9 commits March 5, 2022 18:54

Number conversion using custom bases functionalities and few tests

a93cdf4

Functionalities to format numbers using any base and dictionary

de39717

Add new language text

70910f3

Change models to fit new features

57901eb

Add new communication messages between number converter view models

583d6bc

Add views and functionalities to allow custom dictionaries

9ef907f

Activate model to receive messages

fb4b387

Include language generated component

e678214

Rebase and cleanup code

5a03d4d

youkai95 force-pushed the custom-number-base branch from dde1cf8 to 5a03d4d Compare March 5, 2022 23:58

youkai95 added 4 commits March 5, 2022 19:18

Update InfoBar message to show/hide it correctly

59f028e

Remove remaining code generating compile errors

5ea1eaa

Code refactoring

3e6308f

Fix test cases

59cc700

youkai95 marked this pull request as ready for review March 6, 2022 01:03

youkai95 commented Mar 6, 2022

View reviewed changes

Added the possibility to Favorite/Unfavorite a tool. (DevToys-app#437)

ed2ae5e

* Added the possibility to Favorite/Unfavorite a tool. * Updated localization * addressed feedback

veler requested changes Mar 12, 2022

View reviewed changes

kspc1000 and others added 3 commits March 12, 2022 11:38

Fix README.md minor typo (DevToys-app#441)

a791c4d

Update japanese translation. (DevToys-app#444)

8220d18

Bumped up version number

232b783

Guzzter and others added 3 commits March 14, 2022 23:02

Feature/add refresh button lorem ipsum (DevToys-app#413)

b56d954

* Add StartWithLoremIpsum option * Updated language bundles * Added refresh buttom for lorem ipsum * Replaced click handler with Refresh ICommand * Fixing error Co-authored-by: Guus Beltman <[email protected]>

Added some assets

e6e6159

Merge branch 'main' of https://github.com/veler/DevToys

9625dae

veler and others added 5 commits March 17, 2022 20:29

Added String Escape / Unescape tool (DevToys-app#446)

46fb969

Set nullable

405d68f

Set nullable and change to string dictionaries

faf085c

Fix errors, inject view models and improve code

1799583

Add translations

c9ec26b

youkai95 added 19 commits March 21, 2022 22:06

Number conversion using custom bases functionalities and few tests

a4d5e1a

Functionalities to format numbers using any base and dictionary

88f1e5a

Add new language text

85d9912

Change models to fit new features

ee92b4b

Add new communication messages between number converter view models

7d9b4d2

Add views and functionalities to allow custom dictionaries

35cd720

Activate model to receive messages

c98f7e6

Include language generated component

9ef1138

Rebase and cleanup code

37217dd

Update InfoBar message to show/hide it correctly

84b979e

Remove remaining code generating compile errors

aef2f5d

Code refactoring

6ad9e73

Fix test cases

ea95e96

Set nullable

e6771ba

Set nullable and change to string dictionaries

b0860ec

Fix errors, inject view models and improve code

675778c

Add translations

14bb028

Remove commented code

0553333

Automatic input when navigating to the tool

3d21e10

veler approved these changes Mar 26, 2022

View reviewed changes

veler merged commit cf421ab into DevToys-app:main Mar 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Format from/to custom number base #430

Format from/to custom number base #430

youkai95 commented Feb 27, 2022 •

edited

Loading

youkai95 Mar 6, 2022

veler Mar 12, 2022

youkai95 Mar 12, 2022

veler Mar 13, 2022

youkai95 Mar 23, 2022

youkai95 commented Mar 8, 2022

veler commented Mar 12, 2022

veler commented Mar 12, 2022 •

edited

Loading

veler left a comment

youkai95 commented Mar 13, 2022

youkai95 commented Mar 21, 2022 •

edited

Loading

veler commented Mar 21, 2022

youkai95 commented Mar 21, 2022

veler left a comment

Format from/to custom number base #430

Format from/to custom number base #430

Conversation

youkai95 commented Feb 27, 2022 • edited Loading

Pull request type

What is the current behavior?

What is the new behavior?

Other information

Quality check

youkai95 Mar 6, 2022

Choose a reason for hiding this comment

veler Mar 12, 2022

Choose a reason for hiding this comment

youkai95 Mar 12, 2022

Choose a reason for hiding this comment

veler Mar 13, 2022

Choose a reason for hiding this comment

youkai95 Mar 23, 2022

Choose a reason for hiding this comment

youkai95 commented Mar 8, 2022

veler commented Mar 12, 2022

veler commented Mar 12, 2022 • edited Loading

veler left a comment

Choose a reason for hiding this comment

youkai95 commented Mar 13, 2022

youkai95 commented Mar 21, 2022 • edited Loading

veler commented Mar 21, 2022

youkai95 commented Mar 21, 2022

veler left a comment

Choose a reason for hiding this comment

youkai95 commented Feb 27, 2022 •

edited

Loading

veler commented Mar 12, 2022 •

edited

Loading

youkai95 commented Mar 21, 2022 •

edited

Loading