Improve readme and CDT Cloud update logo #109

planger · 2024-03-15T16:19:00Z

What it does

This change refocuses the README to better resonate with potential users by presenting the extension's features and usage more clearly. We aim to maintain essential information for contributors, ensuring a balance between user orientation and developer guidance.

Also we update the CDT Cloud logo, fixed broken links, and adapted wrong information in the CONTRIBUTING guide left over from Theia.

How to test

Note that the link to the screenshot only works after this is merged. Otherwise we would need to use a relative link, which may not work when displayed in the marketplace or on open-vsx.org.

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the review guidelines

This change refocuses the README to better resonate with potential users by presenting the extension's features and usage more clearly. We aim to maintain essential information for contributors, ensuring a balance between user orientation and developer guidance. Also we update the CDT Cloud logo, fixed broken links, and adapted wrong information in the CONTRIBUTING guide left over from Theia.

README.md

colin-grant-work · 2024-03-15T20:52:18Z

CONTRIBUTING.md

+merged and maintained as part of this project, you should talk with the team
+through an issue. Simply choose the issue you would want to work on, and tell everyone


I got confused here by the idiom 'talk through something.' Maybe

Suggested change

merged and maintained as part of this project, you should talk with the team

through an issue. Simply choose the issue you would want to work on, and tell everyone

merged and maintained as part of this project, you should talk with the team

by posting on an issue. Simply choose the issue you would want to work on, and tell everyone

colin-grant-work · 2024-03-15T20:53:35Z

.github/PULL_REQUEST_TEMPLATE.md


 #### Reminder for reviewers

- as a reviewer, I agree to behave in accordance with [the review guidelines](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#reviewing)
+- as a reviewer, I agree to behave in accordance with [the code of conduct](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#reviewing)


Is there any similar text we could link to outside of the theia-ide organization?

Darn, I meant to point to the CoC of this repo. Thanks for catching that!

colin-grant-work · 2024-03-15T20:54:30Z

README.md

+- **Multiple Format Display**: Shows memory data in different formats, provided your debug adapter supports the `ReadMemoryRequest`.
+- **Address Navigation**: Easily jump to and scroll through memory addresses.
+- **Variable Highlights**: Marks variable memory ranges.
+- **Edit Memory**: Allows in-place memory editing, if the debug adapter supports the `WriteMemoryRequest`.


Let's get #108 in to make this completely true :-).

Yeah, you are right. I'm an optimist, but I'm completely fine to leave this PR open until #108 is merged.

I am sure it will come for the next release one or the other way. ;-)

planger · 2024-03-15T21:55:07Z

Thank you for the fast review @colin-grant-work! I accepted your suggestion and fixed the link. Happy to leave this open until #108 is merged to not promise something before it is actually merged :-)

planger · 2024-03-15T21:55:47Z

Also, I'd love to get @jreineckearm's feedback anyway.

.github/PULL_REQUEST_TEMPLATE.md

jreineckearm

Looks good! A couple of comments.

jreineckearm · 2024-03-18T08:55:49Z

CONTRIBUTING.md

-can discuss it.  If such a feature request already exists, please add a comment
+You may want to see a feature or have an idea. You can
+[file a request](https://github.com/eclipse-cdt-cloud/vscode-memory-inspector/issues/new/choose)
+and we can discuss it. If such a feature request already exists, please add a comment
 or some other form of feedback to indicate you are interested too. Also in this


Suggested change

or some other form of feedback to indicate you are interested too. Also in this

or some other form of feedback to indicate you are interested, too. Also in this

Nit

jreineckearm · 2024-03-18T09:03:01Z

README.md


-## The Memory Provider
+- **Multiple Format Display**: Shows memory data in different formats, provided your debug adapter supports the `ReadMemoryRequest`.


Multiple Format: I don't think we have that yet in the data contents area. Maybe name it Configurable Memory Display and mention the Multiple Format Display for hover-over?

jreineckearm · 2024-03-18T09:03:43Z

README.md

-## The Memory Provider
+- **Multiple Format Display**: Shows memory data in different formats, provided your debug adapter supports the `ReadMemoryRequest`.
+- **Address Navigation**: Easily jump to and scroll through memory addresses.
+- **Variable Highlights**: Marks variable memory ranges.


Suggested change

- **Variable Highlights**: Marks variable memory ranges.

- **Variable Highlights**: Colors memory ranges for variables.

jreineckearm · 2024-03-18T09:04:22Z

README.md

+- **Multiple Format Display**: Shows memory data in different formats, provided your debug adapter supports the `ReadMemoryRequest`.
+- **Address Navigation**: Easily jump to and scroll through memory addresses.
+- **Variable Highlights**: Marks variable memory ranges.
+- **Edit Memory**: Allows in-place memory editing, if the debug adapter supports the `WriteMemoryRequest`.


I am sure it will come for the next release one or the other way. ;-)

jreineckearm · 2024-03-18T09:05:57Z

README.md

+- **Address Navigation**: Easily jump to and scroll through memory addresses.
+- **Variable Highlights**: Marks variable memory ranges.
+- **Edit Memory**: Allows in-place memory editing, if the debug adapter supports the `WriteMemoryRequest`.
+- **Memory Management**: Enables saving and restoring memory data for specific address ranges.


Should we explicitly mention the file format?

jreineckearm · 2024-03-18T09:08:22Z

README.md

+- **Variable Highlights**: Marks variable memory ranges.
+- **Edit Memory**: Allows in-place memory editing, if the debug adapter supports the `WriteMemoryRequest`.
+- **Memory Management**: Enables saving and restoring memory data for specific address ranges.
+- **Custom Views**: Create and customize as many memory views as you need.


Suggested change

- **Custom Views**: Create and customize as many memory views as you need.

- **Customized Views**: Create and customize as many memory views as you need.

jreineckearm · 2024-03-18T09:11:17Z

README.md

+## Getting Started
+
+1. **Install**: Add the extension to VS Code.
+2. **Debug Session**: Start with a debug adapter that has the [`ReadMemory` request](https://microsoft.github.io/debug-adapter-protocol/specification#Requests_ReadMemory) capability.


We should point users to the global Debug Types setting to enable this for any adapter that supports the ReadMemory request.

planger · 2024-03-18T10:49:25Z

@jreineckearm Thank you for your feedback! I think I addressed all your comments and accepted all suggestions.

jreineckearm · 2024-03-18T17:28:02Z

Thanks for the updates! Looks good to me!

planger · 2024-03-18T19:51:23Z

Thanks everyone for their review and suggestions! @colin-grant-work Can you please take another look and, if you're fine with this change, give your approval? Thank you!
I'll wait for #108 to be merged before I merge this change.

jreineckearm · 2024-03-19T20:08:07Z

@planger , @colin-grant-work , as discussed today we should add a terminology section and a screenshot to highlight some of the terminology usage. See #100 (comment) for a summary of that call.

I'd recommend to use this PR for those additions. I am happy to draft something and add it here. But it's getting late now. I'll try to provide a write up tomorrow.

jreineckearm · 2024-03-20T14:57:41Z

Apologies for taking longer than expected. The next comment is a proposal for the wording between "Configuration" and "Contributing". I'll prepare a screenshot next.

jreineckearm · 2024-03-20T14:57:52Z

Memory Format Settings

The Memory Format settings allow to configure how data that is read from the target system is interpreted and displayed.
Use the following to retarget the view to your needs and the inspected memory architecture:

Bytes per MAU: The number of Bytes that form the Minimum Addressable Unit. It commonly is fixed number for a specific target hardware. Use for example a value of 1 for byte-addressable architectures.
MAUs per Group: The number of MAUs that form a Group considering the selected Endianess. Use for example a value of 2 to form a 4-byte value consisting of 2 2-byte MAUs.
Groups per Row: Number of Groups to display in a row. This can be a fixed number of Groups. Or the value Autofit to let the Memory Inspector calculate the best utilization of space in the Data column.
Group Endianess: The order of MAUs within a Group. The value can be Little Endian or Big Endian.

The following terminology is used:

Byte: A data unit of 8 Bits.
MAU (Minimum Addressable Unit): A Minimum Addressable Unit of memory. It consists of one or more Bytes represented by a single address.
Group: A group of MAUs that forms a data value. Groups are the Memory Inspector's default granularity to edit memory contents.
Row: A row in the Memory Inspector display containing multiple Groups.
Endianess: The order of displaying MAUs within a Group.

jreineckearm · 2024-03-20T17:05:29Z

Realized now that we should wait with a screenshot until we have the terminology change PR in.

planger · 2024-03-21T09:12:16Z

Thanks @jreineckearm! I've added your section to the readme and also updated the screenshot to how it'll likely look like after renaming words to MAUs.

jreineckearm · 2024-03-21T09:17:00Z

Thanks @planger , may need to do another review iteration of the text and formatting. Already spotted at least one typo in the text I provided. :-)
The link to the screenshot seems to be broken on the branch. Gives me a 404 when I click it.

planger · 2024-03-21T09:21:40Z

The link to the screenshot seems to be broken on the branch. Gives me a 404 when I click it.

Yes, this link only works after we've merged this change as the image then should become available under the given URL. I preferred to use absolute URLs over relative ones, as this should be safer than something relative when showing the screenshot in the readme of the published extension on the VSX registries.

Edit: but the link actually was wrongly pointing to master even though the branch is main in this repo. Fixed with 2f8d50b.

Just realized the branch is going to be named `main` in this repo.

jreineckearm

Found a tiny typo. Waiting for #111 before taking a screenshot to highlight the Memory Format option impact.

README.md

Co-authored-by: Jens Reinecke <[email protected]>

jreineckearm · 2024-03-22T16:36:03Z

@planger , could you please let me know if attached (modified) screenshot works as a visual reference for the various options?
Unfortunately, a little harder to explain endianess in a single one and wouldn't want to put too many screenshots into one sections.

planger · 2024-03-22T19:17:05Z

@jreineckearm I like it! Thank you!
Just two suggestions for minor tweaks:

It could help to not use Autofit in this screenshot but set it to a concrete value (e.g. 2). This would make it easy to understand, given that there would then be the same number of groups in a row (e.g. 2).
I'm myself not sure about this one, but we could put an example in the text (enumeration in the readme) for the Group Endianess setting. This example could match a specific group in the screenshot to show how the row would be like with Little Endian and Big Endian.

jreineckearm · 2024-03-22T20:05:30Z

Thanks for the feedback, @planger !
Agree with both items. Hope those two images work better. :-)

jreineckearm · 2024-03-22T20:08:10Z

And another one with the item number. But probably a bit too much.

Co-authored-by: Jens Reinecke <[email protected]>

jreineckearm

Thanks! Looks good to me. :-)

planger · 2024-03-25T14:33:37Z

@jreineckearm Thank you for the images! Imho they look great. I've now used the one with the number regarding endianness. Imho it is more consistent, but I don't have a strong opinion. I cropped the first image a bit to better align both screenshots and avoid a light border.

As you can't preview it on Github because of the absolute link, I put a screenshot below:

Please let me know what you think!

jreineckearm · 2024-03-25T14:37:08Z

I cropped the first image a bit to better align both screenshots and avoid a light border.

Thanks! I must have missed that border. IMO good to go with that.

planger · 2024-03-25T14:37:57Z

Thanks @jreineckearm!

colin-grant-work · 2024-03-25T14:38:40Z

My approval stands - merge at your convenience.

arekzaluski reviewed Mar 15, 2024

View reviewed changes

README.md Show resolved Hide resolved

Remove references to Theia in PR template

4b91265

colin-grant-work reviewed Mar 15, 2024

View reviewed changes

Apply review suggestions

0090257

jreineckearm reviewed Mar 18, 2024

View reviewed changes

.github/PULL_REQUEST_TEMPLATE.md Show resolved Hide resolved

jreineckearm reviewed Mar 18, 2024

View reviewed changes

Address review feedback

2e6a174

Fix link to CoC

09129d1

planger requested a review from colin-grant-work March 18, 2024 19:51

colin-grant-work approved these changes Mar 18, 2024

View reviewed changes

jreineckearm mentioned this pull request Mar 19, 2024

Byte-addressable vs word-addressable memory architectures and terminology #100

Closed

Update screenshot and add settings doc

69b5eee

Fix link

2f8d50b

Just realized the branch is going to be named `main` in this repo.

planger mentioned this pull request Mar 21, 2024

Rename word to MAU and add link to settings docs #111

Merged

1 task

jreineckearm reviewed Mar 22, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

Fix type

df8836b

Co-authored-by: Jens Reinecke <[email protected]>

planger and others added 2 commits March 25, 2024 15:16

Add screenshots for options

e9a5406

Co-authored-by: Jens Reinecke <[email protected]>

Crop image to better align

609a47b

jreineckearm approved these changes Mar 25, 2024

View reviewed changes

planger merged commit 950ee01 into eclipse-cdt-cloud:main Mar 25, 2024
5 checks passed

planger deleted the improve-readme branch March 25, 2024 14:52

planger mentioned this pull request Mar 25, 2024

Improve readme #103

Closed

		merged and maintained as part of this project, you should talk with the team
		through an issue. Simply choose the issue you would want to work on, and tell everyone

	or some other form of feedback to indicate you are interested too. Also in this
	or some other form of feedback to indicate you are interested, too. Also in this


		## The Memory Provider
		- Multiple Format Display: Shows memory data in different formats, provided your debug adapter supports the `ReadMemoryRequest`.

	- Variable Highlights: Marks variable memory ranges.
	- Variable Highlights: Colors memory ranges for variables.

	- Custom Views: Create and customize as many memory views as you need.
	- Customized Views: Create and customize as many memory views as you need.

Improve readme and CDT Cloud update logo #109

Improve readme and CDT Cloud update logo #109

Conversation

planger commented Mar 15, 2024

What it does

How to test

Review checklist

Reminder for reviewers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

planger commented Mar 15, 2024

planger commented Mar 15, 2024

jreineckearm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

planger commented Mar 18, 2024

jreineckearm commented Mar 18, 2024

planger commented Mar 18, 2024

jreineckearm commented Mar 19, 2024 • edited Loading

jreineckearm commented Mar 20, 2024

jreineckearm commented Mar 20, 2024

Memory Format Settings

jreineckearm commented Mar 20, 2024

planger commented Mar 21, 2024

jreineckearm commented Mar 21, 2024

planger commented Mar 21, 2024 • edited Loading

jreineckearm left a comment

Choose a reason for hiding this comment

jreineckearm commented Mar 22, 2024

planger commented Mar 22, 2024

jreineckearm commented Mar 22, 2024

jreineckearm commented Mar 22, 2024

jreineckearm left a comment

Choose a reason for hiding this comment

planger commented Mar 25, 2024

jreineckearm commented Mar 25, 2024

planger commented Mar 25, 2024

colin-grant-work commented Mar 25, 2024

jreineckearm commented Mar 19, 2024 •

edited

Loading

planger commented Mar 21, 2024 •

edited

Loading