Skip to content
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

on fix action support comment for group #749

Open
m47730 opened this issue Jan 30, 2024 · 6 comments
Open

on fix action support comment for group #749

m47730 opened this issue Jan 30, 2024 · 6 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@m47730
Copy link

m47730 commented Jan 30, 2024

Hi,
first of all thanks for this tool, i love it!

I struggle with this: if i try to automatically fix an unsorted env files with groups with comment as delimiter

for example:

ENV2=bbb
ENV1=aaa

###> group1 ###
ENV4=ddd
ENV5=eee
ENV3=ccc
###< group1 ###

ENV7=ggg
ENV6=fff

ends up with a sorted list but some key go outside the "comment group" (see ENV3):

ENV1=aaa
ENV2=bbb

ENV3=ccc
###> group1 ###
ENV4=ddd
ENV5=eee
###< group1 ###

ENV6=fff
ENV7=ggg

instead i'd like this output:

ENV1=aaa
ENV2=bbb

###> group1 ###
ENV3=ccc
ENV4=ddd
ENV5=eee
###< group1 ###

ENV6=fff
ENV7=ggg
@mgrachev
Copy link
Member

@m47730 Hi 👋

Thank for your feedback! It's a bug, we'll fix it.

@mgrachev mgrachev added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Feb 10, 2024
@DDtKey
Copy link
Member

DDtKey commented Feb 10, 2024

@mgrachev I don't think it's a bug (in pure form at least)

Comment belongs to ENV variable and not to group. AFAIR it was expected behavior.

###> group1 ### - here could be a comment specific for this env
ENV4=ddd

So. we keep it after reordering for the initial ENV variable. Otherwise how to understand if comment belongs to group or env variable?

But we could consider to keep comments in case of empty line above or below the comment, probably with ability to control the behavior 🤔

Btw, this may work as workaround:

###> group1 ### - there is empty line before and after, we shouldn't change it

ENV4=ddd
ENV5=eee
ENV3=ccc

###< group1 ###

@m47730
Copy link
Author

m47730 commented Feb 13, 2024

Thank you for the workaround (it works as expected)

Could I suggest an option for unbind the comment to next line?
Something like --do-not-move-comments or similiar

@DDtKey
Copy link
Member

DDtKey commented Feb 13, 2024

I think it makes sense to have an argument for this behavior
As an alternative --preserve-comments-position

@reubenborrego
Copy link

Saw on your repo description that you're open to newcomers. I'm about as green to group development as can be. Though I do have plenty of experience in Rust and development overall (And plenty of time to spare).

I wouldn't mind trying to add this QOL improvement. All I need is a bit of upfront guidance and I could probably throw it together. In the meantime I'll keep poking through the repo until I feel confident enough to try something on my own.

@reubenborrego
Copy link

reubenborrego commented Feb 19, 2024

The comment appears to be moved in UnorderedKeyFixer's sort_part function. Altering the the !line.is_comment() condition seemed to work well enough.

Though I believe that's the end of the line for me. Pulling through a command line argument to that point seems to require some degree of restructuring/better understanding of the code base. I'm willing to try but I'd rather not circumvent any current design decisions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Development

No branches or pull requests

4 participants