[GH-ISSUE #24 ] Goodhosts&#39; rewrite of the hosts file renders MAMP incapable of managing its entries #13

Author

Owner

@luthermonson commented on GitHub (Dec 28, 2020):

i dont think you can use the --clean functionality with a rig like mamp then, the only way to get here is through a clean flag right? i am not missing something that got you here via other use?

@luthermonson commented on GitHub (Dec 28, 2020): i dont think you can use the `--clean` functionality with a rig like mamp then, the only way to get here is through a clean flag right? i am not missing something that got you here via other use?

kerem commented

Author

Owner

@tomjn commented on GitHub (Dec 28, 2020):

I think this needs migrating to the vagrant repo at https://github.com/goodhosts/vagrant

@tomjn commented on GitHub (Dec 28, 2020): I think this needs migrating to the `vagrant` repo at https://github.com/goodhosts/vagrant

kerem commented

Author

Owner

@luthermonson commented on GitHub (Dec 28, 2020):

ohhh that makes sense, ill copy/paste and link it and close this

@luthermonson commented on GitHub (Dec 28, 2020): ohhh that makes sense, ill copy/paste and link it and close this

kerem commented

Author

Owner

@Mte90 commented on GitHub (Dec 29, 2020):

To me the issue is not just the plugin that does a clean but the fact that the clean command, cleans all the file and just the parts that are involved with goodhosts.

Clean command was implemented to avoid multiple injection of the same rule by goodhosts as a previous ticket so is a problem in both the projects @luthermonson

@Mte90 commented on GitHub (Dec 29, 2020): To me the issue is not just the plugin that does a clean but the fact that the clean command, cleans all the file and just the parts that are involved with goodhosts. Clean command was implemented to avoid multiple injection of the same rule by goodhosts as a previous ticket so is a problem in both the projects @luthermonson

kerem commented

Author

Owner

@luthermonson commented on GitHub (Dec 29, 2020):

you think there should be a #goodhosts comment or something and just keep everything 100% separate from everyone else?

@luthermonson commented on GitHub (Dec 29, 2020): you think there should be a `#goodhosts` comment or something and just keep everything 100% separate from everyone else?

kerem commented

Author

Owner

@Mte90 commented on GitHub (Dec 29, 2020):

yes in this way the user can use clean without worrying.

@Mte90 commented on GitHub (Dec 29, 2020): yes in this way the user can use clean without worrying.

kerem commented

Author

Owner

@luthermonson commented on GitHub (Dec 29, 2020):

@Mte90 @theMikeD let's do it, if we do some sort of #MANAGED BY GOODHOSTS comment block and only change entries in there it would make the most sense but i feel this requires a 1.1.x version bump, agree?

@luthermonson commented on GitHub (Dec 29, 2020): @Mte90 @theMikeD let's do it, if we do some sort of `#MANAGED BY GOODHOSTS` comment block and only change entries in there it would make the most sense but i feel this requires a 1.1.x version bump, agree?

kerem commented

Author

Owner

@Mte90 commented on GitHub (Dec 29, 2020):

Yes to me is good so I can do a new release of the plugin with that and add a flag to enable or not the clean at all.

@Mte90 commented on GitHub (Dec 29, 2020): Yes to me is good so I can do a new release of the plugin with that and add a flag to enable or not the clean at all.

kerem commented

Author

Owner

@tomjn commented on GitHub (Dec 29, 2020):

This sounds good!

@tomjn commented on GitHub (Dec 29, 2020): This sounds good!

kerem commented

Author

Owner

@theMikeD commented on GitHub (Jan 9, 2021):

Whats the next step?

@theMikeD commented on GitHub (Jan 9, 2021): Whats the next step?

kerem commented

Author

Owner

@luthermonson commented on GitHub (Jan 11, 2021):

find time to add the feature :D PRs welcome~

@luthermonson commented on GitHub (Jan 11, 2021): find time to add the feature :D PRs welcome~

kerem commented

Author

Owner

@tomjn commented on GitHub (Jan 11, 2021):

Luther I think the problem is less time and willingness and more that they
don’t know how to do it, I know that’s definitely the case here.

What would such a PR look like?

On Mon, 11 Jan 2021 at 20:22, Luther Monson notifications@github.com
wrote:

find time to add the feature :D PRs welcome~

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/goodhosts/cli/issues/24#issuecomment-758200613, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAOLZ55NORZI7TLYW4ORJLSZNMYLANCNFSM4VMGYQEQ
.

@tomjn commented on GitHub (Jan 11, 2021): Luther I think the problem is less time and willingness and more that they don’t know how to do it, I know that’s definitely the case here. What would such a PR look like? On Mon, 11 Jan 2021 at 20:22, Luther Monson <notifications@github.com> wrote: > find time to add the feature :D PRs welcome~ > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <https://github.com/goodhosts/cli/issues/24#issuecomment-758200613>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAAOLZ55NORZI7TLYW4ORJLSZNMYLANCNFSM4VMGYQEQ> > . >

kerem commented

Author

Owner

@tomjn commented on GitHub (Jan 13, 2021):

@luthermonson any update or technical direction? You have willing contributors with the time to make the change, who don't know what to do. I'd really like to be able to write a PR but what would that PR look like?

@tomjn commented on GitHub (Jan 13, 2021): @luthermonson any update or technical direction? You have willing contributors with the time to make the change, who don't know what to do. I'd really like to be able to write a PR but what would that PR look like?

kerem commented

Author

Owner

@luthermonson commented on GitHub (Jan 13, 2021):

@tomjn this is what i want.. open to feedback

all code for doing this should be in the cli, the hostsfile package should just be to parse a hosts file to types. this is logic around what to do with the hostsfile after that is done so this code should be here in this repo, can try doing a first round of this have a line type that uses hostsfile.Line and includes a bool which is we check if we manage the line or not.
easiest thing is probably a string to match in comment per line like MAMP does, for example as long as it contains goodhosts in the comment we assume it was meant for our cli, just need a something simple like strings.Contains("goodhosts", comment). e.g. we'd manage all of these lines

127.0.0.1 myhost anotherhost #goodhosts
192.168.0.2 dev-server1 #my comment here goodhosts
192.168.0.3 dev-server2 #goodhosts my comment here

make sure when we add an entry with the add subcommand we need to make sure the comment gets put in place as well
might find it annoying if the goodhosts managed lines are all over the file, might want clean to group them together
dont forget the tests!

@luthermonson commented on GitHub (Jan 13, 2021): @tomjn this is what i want.. open to feedback 1. all code for doing this should be in the cli, the hostsfile package should just be to parse a hosts file to types. this is logic around what to do with the hostsfile after that is done so this code should be here in this repo, can try doing a first round of this have a line type that uses hostsfile.Line and includes a bool which is we check if we manage the line or not. 2. easiest thing is probably a string to match in comment per line like MAMP does, for example as long as it contains `goodhosts` in the comment we assume it was meant for our cli, just need a something simple like `strings.Contains("goodhosts", comment)`. e.g. we'd manage all of these lines ``` 127.0.0.1 myhost anotherhost #goodhosts 192.168.0.2 dev-server1 #my comment here goodhosts 192.168.0.3 dev-server2 #goodhosts my comment here ``` 3. make sure when we add an entry with the add subcommand we need to make sure the comment gets put in place as well 4. might find it annoying if the goodhosts managed lines are all over the file, might want clean to group them together 5. dont forget the tests!

kerem commented

Author

Owner

@theMikeD commented on GitHub (Jan 15, 2021):

Maybe we can just revert the clean imperative until there is a way to turn it off?

@theMikeD commented on GitHub (Jan 15, 2021): Maybe we can just revert the clean imperative until there is a way to turn it off?

kerem commented

Author

Owner

@tomjn commented on GitHub (Jan 16, 2021):

@theMikeD that doesn't make sense in the context of this repo, for Goodhosts CLI, it would be easier to not use --clean than it would be to remove the feature.

I think there's been some misunderstanding as to what goodhosts is on your part:

goodhosts cli, a golang command line program for adding and removing things from the hosts file
vagrant-goodhosts, a vagrant plugin that uses goodhosts to add hosts as requested by a vagrant project
VVV, a vagrant project that uses vagrant goodhosts in place of vagrant hostsupdater

This is the goodhosts cli repo, so reverting the --clean parameter means removing that feature. Unless you meant make vagrant-goodhosts not specify --clean? Because goodhosts only cleans when the --clean parameter is passed

@tomjn commented on GitHub (Jan 16, 2021): @theMikeD that doesn't make sense in the context of this repo, for Goodhosts CLI, it would be easier to not use `--clean` than it would be to remove the feature. I think there's been some misunderstanding as to what goodhosts is on your part: - **goodhosts cli**, a golang command line program for adding and removing things from the hosts file - **vagrant-goodhosts**, a vagrant plugin that uses goodhosts to add hosts as requested by a vagrant project - **VVV**, a vagrant project that uses vagrant goodhosts in place of vagrant hostsupdater This is the goodhosts cli repo, so reverting the `--clean` parameter means removing that feature. Unless you meant make `vagrant-goodhosts` not specify `--clean`? Because goodhosts only cleans when the `--clean` parameter is passed

kerem commented

Author

Owner

@theMikeD commented on GitHub (Jan 16, 2021):

I'm becoming a pro at posting bugs to the wrong repo. Please move this issue elsewhere if you deem it appropriate.

Unless you meant make vagrant-goodhosts not specify --clean?

Yes, this. As I understand it, doing so isn't something I can control with a config file.

@theMikeD commented on GitHub (Jan 16, 2021): I'm becoming a pro at posting bugs to the wrong repo. Please move this issue elsewhere if you deem it appropriate. > Unless you meant make `vagrant-goodhosts` not specify `--clean`? Yes, this. As I understand it, doing so isn't something I can control with a config file.

kerem commented

Author

Owner

@tomjn commented on GitHub (Jan 16, 2021):

You posted in the right repo, but that particular request was for a different piece of software. IIRC @Mte90 has already fixed and merged a PR that makes it configurable and off by default

@tomjn commented on GitHub (Jan 16, 2021): You posted in the right repo, but that particular request was for a different piece of software. IIRC @Mte90 has already fixed and merged a PR that makes it configurable and off by default

kerem commented

Author

Owner

@tomjn commented on GitHub (Jan 16, 2021):

As for this feature, I think these semantics make sense if the CLI is used in conjunction with a --namespace="" parameter.

When adding a host:

if the host already exists
- if it exists on a line without the namespace comment on the end
  - error message + bad error code
- if it exists on a line with the namespace comment
  - remove from file and add on a new line if the IP is different with namespace comment
  - do nothing if the IP is the same
if it does not exist
- add on a new line with namespace comment

When removing a host:

if it exists on a line without the namespace comment on the end
- error message + bad error code
if it exists on a line with the namespace comment
- remove from file

Additionally, the namespace parameter would allow rival namespaces, as well as the ability for us to do things like:

goodhosts remove cnmd.test --namespace="MAMP PRO - Do NOT remove this entry!"

@tomjn commented on GitHub (Jan 16, 2021): As for this feature, I think these semantics make sense if the CLI is used in conjunction with a `--namespace=""` parameter. When adding a host: - if the host already exists - if it exists on a line without the namespace comment on the end - error message + bad error code - if it exists on a line with the namespace comment - remove from file and add on a new line if the IP is different with namespace comment - do nothing if the IP is the same - if it does not exist - add on a new line with namespace comment When removing a host: - if it exists on a line without the namespace comment on the end - error message + bad error code - if it exists on a line with the namespace comment - remove from file Additionally, the namespace parameter would allow rival namespaces, as well as the ability for us to do things like: ```sh goodhosts remove cnmd.test --namespace="MAMP PRO - Do NOT remove this entry!" ```

kerem commented

Author

Owner

@tomjn commented on GitHub (Jan 16, 2021):

all code for doing this should be in the cli, the hostsfile package should just be to parse a hosts file to types. this is logic around what to do with the hostsfile after that is done so this code should be here in this repo, can try doing a first round of this have a line type that uses hostsfile.Line and includes a bool which is we check if we manage the line or not.

This seems sensible

easiest thing is probably a string to match in comment per line like MAMP does, for example as long as it contains goodhosts in the comment we assume it was meant for our cli, just need a something simple like strings.Contains("goodhosts", comment). e.g. we'd manage all of these lines

I think we should allow the namespace to be configurable from the CLI parameter for maximum flexibility, then search for # namespace \n or equivalent

@tomjn commented on GitHub (Jan 16, 2021): > all code for doing this should be in the cli, the hostsfile package should just be to parse a hosts file to types. this is logic around what to do with the hostsfile after that is done so this code should be here in this repo, can try doing a first round of this have a line type that uses hostsfile.Line and includes a bool which is we check if we manage the line or not. This seems sensible > easiest thing is probably a string to match in comment per line like MAMP does, for example as long as it contains goodhosts in the comment we assume it was meant for our cli, just need a something simple like strings.Contains("goodhosts", comment). e.g. we'd manage all of these lines I think we should allow the namespace to be configurable from the CLI parameter for maximum flexibility, then search for `# namespace \n` or equivalent

kerem commented

Author

Owner

@Mte90 commented on GitHub (Feb 22, 2021):

any updates for this @luthermonson?

@Mte90 commented on GitHub (Feb 22, 2021): any updates for this @luthermonson?

kerem commented

Author

Owner

@luthermonson commented on GitHub (Feb 22, 2021):

oh thanks for the ping. this fell off my radar. adding it to my task list and ill try and get to it this week

@luthermonson commented on GitHub (Feb 22, 2021): oh thanks for the ping. this fell off my radar. adding it to my task list and ill try and get to it this week

kerem commented

Author

Owner

@tomjn commented on GitHub (Mar 22, 2021):

@theMikeD latest goodhosts vagrant plugin removed the clean operation a few months ago

@tomjn commented on GitHub (Mar 22, 2021): @theMikeD latest goodhosts vagrant plugin removed the clean operation a few months ago

kerem commented

Author

Owner

@Mte90 commented on GitHub (Apr 2, 2021):

@luthermonson just a ping again :-D

@Mte90 commented on GitHub (Apr 2, 2021): @luthermonson just a ping again :-D

kerem commented

Author

Owner

@luthermonson commented on GitHub (Apr 5, 2021):

oi vey

@luthermonson commented on GitHub (Apr 5, 2021): oi vey

kerem commented

Author

Owner

@Mte90 commented on GitHub (May 11, 2021):

and it is time for another ping @luthermonson :-D

@Mte90 commented on GitHub (May 11, 2021): and it is time for another ping @luthermonson :-D

kerem commented

Author

Owner

@luthermonson commented on GitHub (Jun 3, 2021):

missed this ping... sorry. im bad at getting this feature done.

@luthermonson commented on GitHub (Jun 3, 2021): missed this ping... sorry. im bad at getting this feature done.

kerem commented

Author

Owner

@Mte90 commented on GitHub (Jun 28, 2021):

it's me again :-D
I don't know Go otherwise I will try to implement it.

@Mte90 commented on GitHub (Jun 28, 2021): it's me again :-D I don't know Go otherwise I will try to implement it.

kerem commented