-
Notifications
You must be signed in to change notification settings - Fork 21
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
Reformat Column Comments #117
Comments
@Z2Flow thanks for submitting a detailed issue. I think the request makes sense. I'll have some time to take a look at this on my Friday. |
@Z2Flow Sorry for the delay, I had a time to take a look. Adding support for multi-line column comments should be okay, did you have an example migration you could share where you add new lines? Regarding word wrapping, I would want to flush it out a bit because of the impact of supporting max line width would affect a lot of the code. I did a quick digging as well since I haven't touched anything that does word wrapping and came across this gem [1] and apparently there are different word wrap / soft wrap modes. |
Hi Andrew,
np.
Thanks for getting back to me.
I am a bit of an odd duck in the Rails world.
I use an app “DbSchema" to design my databases.
I am a visual person and find the text-based approach doesn’t work for me.
So, I export the SQL DDL from the DbSchema model and drop it into a migration file.
Here is an example fragment:
class InitialDbStructure < ActiveRecord::Migration[7.1]
def up
execute <<-SQL
CREATE SCHEMA IF NOT EXISTS "public";
CREATE EXTENSION IF NOT EXISTS citext;
CREATE TABLE administrators (
id bigserial NOT NULL ,
created_at timestamptz NOT NULL ,
updated_at timestamptz NOT NULL ,
first_name citext NOT NULL ,
last_name citext NOT NULL ,
After each change I drop the database and recreate it from the new version of the migration file.
I keep each exported DDL file in case of rollback.
If you need an example of a classic Rails migration file, perhaps contacting @mvastola, who commented on the same issue I posted for annotate: ctran/annotate_models#1011
He may be able to provide a better example.
Thanks.
Cheers,
Rob
… On Jun 2, 2024, at 19:18, Andrew W. Lee ***@***.***> wrote:
@Z2Flow <https://github.com/Z2Flow> Sorry for the delay, I had a time to take a look. Adding support for multi-line column comments should be okay, did you have an example migration you could share where you add new lines?
Regarding word wrapping, I would want to flush it out a bit because of the impact of supporting max line width would affect a lot of the code. I did a quick digging as well since I haven't touched anything that does word wrapping and came across this gem [1] and apparently there are different word wrap / soft wrap modes.
[1] https://github.com/pazdera/word_wrap
—
Reply to this email directly, view it on GitHub <#117 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ5VGA3KTHH27WBBL7I6RA3ZFOR5DAVCNFSM6AAAAABIJAJN6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBUGA2TIMRQGI>.
You are receiving this because you were mentioned.
|
Hi Andrew,
I forgot to add that I think the WordWrap gem should work.
Also, here is an example of a SQL COMMENT on a jsonb column:
COMMENT ON COLUMN municipalities.fire_detector IS 'Detailed information if the isolation is fire (Vaughan):
{
serial_number: "",
reading: "",
units: "" - e.g. m3
installed: boolean
}’;
If there’s anything else I can help with, please let me know.
Cheers,
Rob
… On Jun 3, 2024, at 13:38, Quicksilver Cloud ***@***.***> wrote:
Hi Andrew,
np.
Thanks for getting back to me.
I am a bit of an odd duck in the Rails world.
I use an app “DbSchema" to design my databases.
I am a visual person and find the text-based approach doesn’t work for me.
So, I export the SQL DDL from the DbSchema model and drop it into a migration file.
Here is an example fragment:
class InitialDbStructure < ActiveRecord::Migration[7.1]
def up
execute <<-SQL
CREATE SCHEMA IF NOT EXISTS "public";
CREATE EXTENSION IF NOT EXISTS citext;
CREATE TABLE administrators (
id bigserial NOT NULL ,
created_at timestamptz NOT NULL ,
updated_at timestamptz NOT NULL ,
first_name citext NOT NULL ,
last_name citext NOT NULL ,
After each change I drop the database and recreate it from the new version of the migration file.
I keep each exported DDL file in case of rollback.
If you need an example of a classic Rails migration file, perhaps contacting @mvastola, who commented on the same issue I posted for annotate: ctran/annotate_models#1011
He may be able to provide a better example.
Thanks.
Cheers,
Rob
> On Jun 2, 2024, at 19:18, Andrew W. Lee ***@***.***> wrote:
>
>
> @Z2Flow <https://github.com/Z2Flow> Sorry for the delay, I had a time to take a look. Adding support for multi-line column comments should be okay, did you have an example migration you could share where you add new lines?
>
> Regarding word wrapping, I would want to flush it out a bit because of the impact of supporting max line width would affect a lot of the code. I did a quick digging as well since I haven't touched anything that does word wrapping and came across this gem [1] and apparently there are different word wrap / soft wrap modes.
>
> [1] https://github.com/pazdera/word_wrap
>
> —
> Reply to this email directly, view it on GitHub <#117 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ5VGA3KTHH27WBBL7I6RA3ZFOR5DAVCNFSM6AAAAABIJAJN6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBUGA2TIMRQGI>.
> You are receiving this because you were mentioned.
>
|
@Z2Flow apologies for the delay, I have some time now so I'm back to working on multi-line comments. Can you check for me how class AddFooToTestDefaults < ActiveRecord::Migration[7.0]
def change
add_column :test_defaults, :foo, :boolean, default: false, comment: 'The list of causes of failure for an assembly.\n This is a template field copied to the Test when it is created.\n This array creates a list of checkboxes in the form.'
end
end then in the schema and (I think in Postgres as well) it gets escaped. Can you confirm? Snippet from my schema.rb: t.boolean "foo", default: false, comment: "The list of causes of failure for an assembly.\\n This is a template field copied to the Test when it is created.\\n This array creates a list of checkboxes in the form." |
Hi Andrew,
Apologies for the slow reply.
I made this Migration:
class AddFooToPerson < ActiveRecord::Migration[7.2]
def change
add_column :people, :foo, :boolean, default: false, comment: 'The list of causes of failure for an assembly
.\nThis is a template field copied to the Test when it is created.\nThis array creates a list of checkboxes in the
form.'
end
end
and got this output:
# foo(The list of causes of failure for an assembly\n.\nThis is a template field copied to the Test when it is created.\nThis array creates a list of checkboxes in the\nform.:boolean default(FALSE)
Cheers,
Rob
… On Jun 20, 2024, at 20:59, Andrew W. Lee ***@***.***> wrote:
@Z2Flow <https://github.com/Z2Flow> apologies for the delay, I have some time now so I'm back to working on multi-line comments.
Can you check for me how \n (line breaks) are stored for you? I'm finding if I have a migration that adds multi-line comments like this:
class AddFooToTestDefaults < ActiveRecord::Migration[7.0]
def change
add_column :test_defaults, :foo, :boolean, default: false, comment: 'The list of causes of failure for an assembly.\n This is a template field copied to the Test when it is created.\n This array creates a list of checkboxes in the form.'
end
end
then in the schema and (I think in Postgres as well) it gets escaped. Can you confirm?
Snippet from my schema.rb:
t.boolean "foo", default: false, comment: "The list of causes of failure for an assembly.\\n This is a template field copied to the Test when it is created.\\n This array creates a list of checkboxes in the form."
—
Reply to this email directly, view it on GitHub <#117 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ5VGA7DBVD5LO27FKTPCSTZIN3F7AVCNFSM6AAAAABIJAJN6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBRG44TOMJRGQ>.
You are receiving this because you were mentioned.
|
As an update, I did take a look at this yesterday. The changes required were more extensive than I originally thought, so for the time being I'm not going to work on this and instead focus on some refactors to make this change easier in the future. If you do want to add this change though, I would be more than happy to review any PRs. Changes required would be:
Potential changes:
|
Hi Andrew,
Understood.
I am up to my eyeballs at the moment, but will consider doing a PR as a back burner project.
Cheers,
Rob
… On Jun 27, 2024, at 22:20, Andrew W. Lee ***@***.***> wrote:
As an update, I did take a look at this yesterday. The changes required were more extensive than I originally thought, so for the time being I'm not going to work on this and instead focus on some refactors to make this change easier in the future. If you do want to add this change though, I would be more than happy to review any PRs.
Changes required would be:
Adding new config/parser option
ModelWrapper#max_schema_info_width adding support for getting the width if there are column comments and split by new lines
ColumnAnnotation::AnnotationBuilder#build support making the new lines comments using the correct formatter (default, markdown, rdoc, etc)
Potential changes:
AnnotationDiffGenerator may need to be changed
—
Reply to this email directly, view it on GitHub <#117 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZ5VGA626ULIRASLUXREYVTZJTB67AVCNFSM6AAAAABIJAJN6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJVHE4DOMJQGQ>.
You are receiving this because you were mentioned.
|
This stems from reverse-engineering a PostgreSQL database…
Add the ability to move column comments to the next line, and indented 'n' spaces.
Also soft-wrap to column 'm' (default 80), and interpret newline characters (\n).
Currently columns with long comments cause the page width to extend to the end of the comment making it difficult to access the datatype as it overflows the window.
E.g. current formatting:
# assembly_failures(The list of causes of failure for an assembly.\n This is a template field copied to the Test when it is created.\n This array creates a list of checkboxes in the form. :jsonb
Proposed formatting:
Version
The text was updated successfully, but these errors were encountered: